Code10x
SOLIDmedium3-5 years

A `discount(Order o)` method has grown to five `if`/`else if` branches over three sprints, and the newest branch broke a case that was working last week. The team's instinct is "add a comment warning about branch order." What does open/closed say the actual fix is, and when would adding a sixth `else if` still be the right call instead?

A warning comment treats the symptom — this method is fragile to edit — without removing the cause, which is that every new discount requires editing code that already works and re-arguing the order of branches that interact in ways nobody designed for. Open/closed's answer is the refactor to strategy: a DiscountRule interface, one class per rule, and a list (Spring can inject it) that the method iterates — a new discount becomes a new class, and discount() itself never changes again, so there is no shared method left to break. But the lesson is equally clear this isn't free, and with only two branches the plain if is the honest design — it's closed to modification only in the sense that nobody wants to modify it, and refactoring a two-branch method into an interface, a class per branch and an injected list is paying real cost (a file to open, a name to learn) for a problem that doesn't exist yet. The trigger the lesson names explicitly is the third case: that's when a growing conditional and a strategy refactor stop being a coin flip.

The lesson behind it →
PreviousA teammate writes `class ReadOnlyOrderRepository extends JpaOrderRepository { public Order save(Order o) { throw new UnsupportedOperationException(); } }` to enforce that a reporting service can't accidentally write. It compiles cleanly and passes code review. What's actually wrong with it, and what should the fix look like?Next A service does `if (order.getStatus() == OPEN && !order.getLines().isEmpty()) { order.setStatus(PAID); order.setPaymentRef(ref); }` — and the exact same check, with a subtly different condition, lives in three other services. A batch job sets an order's status to `PAID` directly, with no lines, and nobody notices for a week. What's the underlying design problem, and what does the fix actually look like in code?
More on SOLID
A teammate writes `class ReadOnlyOrderRepository extends JpaOrderRepository { public Order save(Order o) { throw new UnsupportedOperationException(); } }` to enforce that a reporting service can't accidentally write. It compiles cleanly and passes code review. What's actually wrong with it, and what should the fix look like?Precisely why can `javac` catch a stronger precondition on an overloaded method's parameter types but not a stronger precondition on an override's *behavior* — and what, exactly, is `invokevirtual` doing at the bytecode level that makes an LSP violation a purely run-time phenomenon?