Clean code & refactoring

I judge clean code by how safely another engineer can understand and change it. I look for meaningful names, cohesive functions, explicit dependencies and visible failure paths. I also want tests around business rules and boundaries, because readability alone does not establish correctness. I do not apply a fixed method length or introduce patterns automatically. A short function with hidden side effects can be harder to maintain than a longer, straightforward calculation.

I choose names that express domain intent and make the call site readable. For example, paymentTimeoutSeconds is clearer than timeout, and ReserveStock is clearer than Process. I use the same term for the same concept throughout the application and distinguish different concepts even when their current implementation matches. I follow C# conventions such as the Async suffix and avoid abbreviations that require local knowledge. If naming is difficult, I check whether the responsibility itself is unclear.

I keep a function focused on one coherent operation and roughly one level of abstraction. An orchestration method can validate an order, reserve inventory and save it, while detailed calculations live behind meaningful names. I do not use a universal line limit because a clear mapping or algorithm may need more lines. I extract a method when it introduces a useful concept or isolates complexity, and avoid splitting code so aggressively that reading it requires constant navigation.

I use guard clauses to handle invalid input and exceptional cases near the beginning of a method, leaving the normal path easy to follow. They reduce indentation and make preconditions visible. During refactoring, I preserve evaluation order, null handling and side effects, because flattening conditions can accidentally change behavior. I also distinguish invalid programming input from expected business rejection. The former may justify an argument exception, while the latter often deserves an explicit result.

I write comments for information the code cannot communicate well, such as a regulatory constraint, a deliberate trade-off or a workaround linked to an issue. I avoid comments that simply repeat the statement, and I rename or extract confusing code before explaining it in prose. Public API documentation can describe contracts and failure behavior. I remove commented-out implementations because version control already preserves history, and I update useful comments whenever the relevant behavior changes.

I interpret DRY as avoiding multiple authoritative representations of the same knowledge. If the same tax rule is implemented in three places, a policy should own it. However, similar-looking code can represent independent rules, such as a loyalty discount and an invoice adjustment. Combining them can create coupling and complicated flags. I first ask whether the pieces must change together, then extract a shared concept when that relationship is real rather than deduplicating purely by appearance.

I use KISS to choose the simplest design that satisfies the actual functional and operational requirements. YAGNI stops me from building extension points for scenarios nobody currently needs. For example, I would not introduce a plugin framework for one shipping calculation. I still preserve useful boundaries and meet security and reliability requirements, because those are real needs. When a second variation appears, I can refactor using evidence instead of carrying a speculative abstraction indefinitely.

Shotgun surgery means one logical change requires coordinated edits across many files, often because a rule is scattered. Divergent change means one class changes for several unrelated reasons, often because it owns too many responsibilities. I address the first by moving related behavior and data together, and the second by extracting cohesive responsibilities. I verify the actual change history before restructuring, because class size or file count alone does not prove either problem.

I consider polymorphism when the same type-based branching appears in several operations and each variant has meaningful behavior. A shipping policy can then own its price and delivery rules, reducing coordinated switch changes. I keep construction or selection at a boundary and reject unsupported values explicitly. For one small switch over a stable enum, introducing a hierarchy may add more complexity than it removes. I choose based on repetition and change patterns rather than treating every switch as a defect.

Tell-don't-ask encourages me to request behavior from the object that owns a rule instead of reading its state and reproducing the rule elsewhere. The Law of Demeter limits dependence on a collaborator's internal object graph. For example, order.Cancel can protect valid transitions, and an order shipping snapshot avoids traversing customer internals. I apply these ideas to reduce coupling and protect invariants, while keeping simple data transfer and query projections straightforward.

I prefer composition when behavior varies independently, such as shipping, discounts or payment providers. Injecting a collaborator lets me replace one policy without inheriting unrelated state or protected implementation details. It also makes dependencies visible and tests more focused. Inheritance remains useful when there is a genuine substitutable relationship and a stable base contract. I avoid deep hierarchies created only for code reuse, because base-class changes can unexpectedly affect every derived class.

I first identify the behavior and contracts that must remain stable, including failure paths and side effects. I add characterization tests around those boundaries, using a small seam if necessary to isolate a dependency. Then I make one structural change at a time and run the relevant tests. I separate discovered bugs from refactoring so reviewers can see intentional behavior changes. For high-risk paths, I also compare production signals and use a gradual rollout with a clear rollback path.

I prioritize technical debt by its effect on delivery, reliability and risk rather than by how old or unattractive the code looks. I record the shortcut, its interest cost, the affected area and a practical repayment trigger. Repeated incidents or changes that consistently touch many files are useful evidence. I favor small improvements alongside related feature work, and propose a larger investment only when its expected benefit and migration risk justify the cost.

I review the intended behavior first, then correctness, security, consistency and test coverage before style. My comments describe a concrete scenario and consequence, and suggest an action or ask a focused question. I label whether something blocks approval or is optional, and explain the trade-off when several solutions are reasonable. I leave mechanical formatting to tooling. If a discussion becomes lengthy, I resolve it directly and record the decision so the author has a clear next step.

I use analyzers to catch repeatable defects and shared conventions, and nullable reference types to make nullability part of the compile-time contract. I can promote nullable warnings to build errors and share formatting or diagnostic severity through .editorconfig. This reduces review noise, but it does not prove the business logic is correct. The null-forgiving operator only suppresses analysis; it does not validate input at runtime. External input still needs validation, and behavior still needs appropriate tests.