diff --git a/README.md b/README.md index a8724aa..c521256 100644 --- a/README.md +++ b/README.md @@ -20,7 +20,7 @@ Every pattern has: | Category | Pattern | Bookshop example | Key benefit | |---|---|---|---| | Creational | [Builder](src/main/java/com/bookshop/creational/builder/README.md) | Assembling an `Order` with optional gift message, promo code, express delivery | Readable construction of immutable objects; no telescoping constructors | -| Creational | [Factory Method](src/main/java/com/bookshop/creational/factorymethod/README.md) | Creating card/PayPal/gift-card processors from a `PaymentMethod` | Callers never couple to concrete classes; new methods touch one place | +| Creational | [Simple Factory](src/main/java/com/bookshop/creational/simplefactory/README.md) | Creating card/PayPal/gift-card processors from a `PaymentMethod` | Callers never couple to concrete classes; a new method is one compiler-checked change | | Creational | [Singleton](src/main/java/com/bookshop/creational/singleton/README.md) | Shop-wide config and feature flags via the enum idiom | Guaranteed single shared instance -- with an honest "prefer DI" caveat | | Structural | [Adapter](src/main/java/com/bookshop/structural/adapter/README.md) | Wrapping a legacy pence-and-status-codes payment gateway behind a clean interface | Incompatible APIs cooperate; the ugliness is quarantined in one class | | Structural | [Decorator](src/main/java/com/bookshop/structural/decorator/README.md) | Stacking gift wrap, express handling and greeting card onto an order | Combine extras at runtime without 2^n subclasses | @@ -67,17 +67,22 @@ mvn verify # lints (Checkstyle) and runs every pattern's tests ## What's deliberately left out -- and why -Half the Gang of Four catalogue rarely appears in modern application code. Excluded on purpose: +This is a curated list, not a complete catalogue: the bar for inclusion is "working Java +developers hand-write this regularly". The rest of the Gang of Four is left out as a scope +choice -- these patterns are real and still appear, just not often enough in ordinary application +code to earn a chapter here: -- **Command** -- its niche (tasks as objects) is covered by `Runnable` and lambdas; hand-rolled - undo stacks are rare. +- **Command** -- when the need is simply "a task as an object", `Runnable` and lambdas usually + carry it; the full pattern (queues, undo stacks, audit logs of operations) is genuinely useful + but comparatively rare. - **State** -- real projects usually reach for an enum + switch or a state-machine library rather than the class-per-state shape. - **Composite** -- common *inside* UI/tree libraries, rarely hand-written in application code. - **Iterator** -- absorbed into the language: `Iterable` and for-each. - **Abstract Factory, Prototype, Bridge, Flyweight, Visitor, Mediator, Memento, Interpreter** -- - superseded by dependency injection, enums and lambdas, or simply too rare to earn a place in a - "patterns you'll actually use" list. + each has its niche (a DI container does much of Abstract Factory's everyday job; Visitor still + shines in compilers and AST tooling), but in typical application code they're rare enough that a + focused list serves you better than a complete one. If you're deciding whether to use a pattern at all: the tests here show the *problem* each pattern earns its keep on. No matching problem, no pattern -- the simplest code that works wins. The full diff --git a/docs/anti-patterns.md b/docs/anti-patterns.md index 8c7a62b..d0e94a8 100644 --- a/docs/anti-patterns.md +++ b/docs/anti-patterns.md @@ -34,9 +34,10 @@ if (status == 3) { refund(order); } // 3 means. **How it happens:** the value was obvious to whoever typed it, that afternoon. **The damage:** the business rule is unfindable and unsearchable; when "free shipping over GBP 25" becomes GBP 30, some of the 25.00s get updated (see [copy-paste programming](#3-copy-paste-programming)); `status == 3` -invites off-by-one bugs no compiler can catch. **The counter:** named constants -(`FREE_SHIPPING_THRESHOLD`) and enums -- -[`PaymentMethod`](../src/main/java/com/bookshop/creational/factorymethod/PaymentMethod.java.md) makes +invites off-by-one bugs no compiler can catch. **The counter:** named constants -- +[`StandardShipping`](../src/main/java/com/bookshop/solid/ocp/StandardShipping.java.md)'s +`FREE_SHIPPING_THRESHOLD` names exactly this rule -- and enums -- +[`PaymentMethod`](../src/main/java/com/bookshop/creational/simplefactory/PaymentMethod.java.md) makes invalid payment types unrepresentable, and the compiler checks every `switch` over it. ## 3. Copy-paste programming diff --git a/docs/use-with-judgement.md b/docs/use-with-judgement.md index 130bc28..1a30c58 100644 --- a/docs/use-with-judgement.md +++ b/docs/use-with-judgement.md @@ -12,7 +12,7 @@ The benefit column in this repo's tables is real, but so is the invoice: | Tool | Solves | Costs when the problem isn't there | |---|---|---| | Builder | Many optional fields | Fifty lines of ceremony for a two-field class -- a constructor was clearer | -| Factory Method | Callers coupled to concrete types | An indirection layer over a single `new` that never varies | +| Simple Factory | Callers coupled to concrete types | An indirection layer over a single `new` that never varies | | Strategy | Multiple interchangeable algorithms | An interface, a context, and one lonely implementation -- an `if` was clearer | | Interfaces everywhere | Multiple implementations / substitution in tests | `BookService` + `BookServiceImpl` pairs that only ever have one member | | SRP | Unrelated reasons-to-change tangled together | Logic shattered into confetti -- ten two-line classes nobody can follow | diff --git a/src/main/java/com/bookshop/behavioral/chainofresponsibility/OrderCheck.java b/src/main/java/com/bookshop/behavioral/chainofresponsibility/OrderCheck.java index 5ccf7d8..a43b01c 100644 --- a/src/main/java/com/bookshop/behavioral/chainofresponsibility/OrderCheck.java +++ b/src/main/java/com/bookshop/behavioral/chainofresponsibility/OrderCheck.java @@ -10,7 +10,15 @@ public abstract class OrderCheck { private OrderCheck next; - /** Links the given checks in order and returns the head of the chain. */ + /** + * Links the given checks in order and returns the head of the chain. + * + *

Linking stores each handler's successor in the handler itself, so a handler + * instance belongs to one chain at a time -- to assemble several pipelines + * from the same parts, build each from fresh instances. Relinking the same + * instances into a new chain is safe: every {@code next} is overwritten, including + * the last handler's, so no tail from a previous, longer chain survives. + */ public static OrderCheck chainOf(List checks) { if (checks.isEmpty()) { throw new IllegalArgumentException("chain needs at least one check"); @@ -18,6 +26,7 @@ public static OrderCheck chainOf(List checks) { for (int i = 0; i < checks.size() - 1; i++) { checks.get(i).next = checks.get(i + 1); } + checks.get(checks.size() - 1).next = null; return checks.get(0); } diff --git a/src/main/java/com/bookshop/behavioral/chainofresponsibility/README.md b/src/main/java/com/bookshop/behavioral/chainofresponsibility/README.md index 11994e4..12464b3 100644 --- a/src/main/java/com/bookshop/behavioral/chainofresponsibility/README.md +++ b/src/main/java/com/bookshop/behavioral/chainofresponsibility/README.md @@ -28,7 +28,10 @@ This is the servlet-filter / middleware shape: the pipeline is *configuration*, - **Short-circuiting for free** -- a rejection stops the chain; later (possibly expensive) checks never run. - **Separation of *what checks exist* from *which run*** -- different channels (web, phone, trade) - can assemble different chains from the same parts. + can each assemble their own chain from the same check *classes*. (One honest limitation of the + linked-handler shape: each handler *instance* stores its successor, so it belongs to one chain + at a time -- give each chain its own instances. An immutable list-based pipeline avoids this at + the cost of a less canonical shape.) ## Seen in the wild diff --git a/src/main/java/com/bookshop/behavioral/templatemethod/CsvSalesReport.java b/src/main/java/com/bookshop/behavioral/templatemethod/CsvSalesReport.java index 2eb2af6..4765e04 100644 --- a/src/main/java/com/bookshop/behavioral/templatemethod/CsvSalesReport.java +++ b/src/main/java/com/bookshop/behavioral/templatemethod/CsvSalesReport.java @@ -3,7 +3,13 @@ import com.bookshop.domain.Book; import java.math.BigDecimal; -/** CSV variant: supplies only the formatting steps, inherits the algorithm. */ +/** + * CSV variant: supplies only the formatting steps, inherits the algorithm. + * + *

Deliberately simplified: real CSV output must quote fields containing commas or + * quotes (RFC 4180) -- use a CSV library for that. The lesson here is the Template + * Method shape, not CSV serialization. + */ public class CsvSalesReport extends SalesReport { @Override diff --git a/src/main/java/com/bookshop/behavioral/templatemethod/HtmlSalesReport.java b/src/main/java/com/bookshop/behavioral/templatemethod/HtmlSalesReport.java index 8128447..86c7370 100644 --- a/src/main/java/com/bookshop/behavioral/templatemethod/HtmlSalesReport.java +++ b/src/main/java/com/bookshop/behavioral/templatemethod/HtmlSalesReport.java @@ -3,7 +3,13 @@ import com.bookshop.domain.Book; import java.math.BigDecimal; -/** HTML variant: different formatting, identical structure and totalling logic. */ +/** + * HTML variant: different formatting, identical structure and totalling logic. + * + *

Deliberately simplified: real HTML output must escape {@code < > &} in field + * values -- use a templating library for that. The lesson here is the Template Method + * shape, not HTML generation. + */ public class HtmlSalesReport extends SalesReport { @Override diff --git a/src/main/java/com/bookshop/behavioral/templatemethod/README.md b/src/main/java/com/bookshop/behavioral/templatemethod/README.md index 28ff105..d8890ce 100644 --- a/src/main/java/com/bookshop/behavioral/templatemethod/README.md +++ b/src/main/java/com/bookshop/behavioral/templatemethod/README.md @@ -25,6 +25,10 @@ String html = new HtmlSalesReport().generate(sales); // different steps never *whether the footer comes last*. - **Adding a format is trivial** -- implement three small methods; the hard part is inherited. +One caveat so nobody copies these as serializers: the formatters are deliberately minimal. Real +CSV needs field quoting (RFC 4180) and real HTML needs escaping -- both jobs for a library. The +lesson here is the *skeleton*, not the formats. + ## A note on modern practice When the base class would have only one hook, prefer passing a lambda (Strategy) instead of diff --git a/src/main/java/com/bookshop/creational/factorymethod/PaymentMethod.java b/src/main/java/com/bookshop/creational/simplefactory/PaymentMethod.java similarity index 72% rename from src/main/java/com/bookshop/creational/factorymethod/PaymentMethod.java rename to src/main/java/com/bookshop/creational/simplefactory/PaymentMethod.java index cee54bd..251dc77 100644 --- a/src/main/java/com/bookshop/creational/factorymethod/PaymentMethod.java +++ b/src/main/java/com/bookshop/creational/simplefactory/PaymentMethod.java @@ -1,4 +1,4 @@ -package com.bookshop.creational.factorymethod; +package com.bookshop.creational.simplefactory; /** The payment options the shop offers at checkout. */ public enum PaymentMethod { diff --git a/src/main/java/com/bookshop/creational/factorymethod/PaymentMethod.java.md b/src/main/java/com/bookshop/creational/simplefactory/PaymentMethod.java.md similarity index 100% rename from src/main/java/com/bookshop/creational/factorymethod/PaymentMethod.java.md rename to src/main/java/com/bookshop/creational/simplefactory/PaymentMethod.java.md diff --git a/src/main/java/com/bookshop/creational/factorymethod/PaymentProcessor.java b/src/main/java/com/bookshop/creational/simplefactory/PaymentProcessor.java similarity index 80% rename from src/main/java/com/bookshop/creational/factorymethod/PaymentProcessor.java rename to src/main/java/com/bookshop/creational/simplefactory/PaymentProcessor.java index 57ad15f..8451aad 100644 --- a/src/main/java/com/bookshop/creational/factorymethod/PaymentProcessor.java +++ b/src/main/java/com/bookshop/creational/simplefactory/PaymentProcessor.java @@ -1,4 +1,4 @@ -package com.bookshop.creational.factorymethod; +package com.bookshop.creational.simplefactory; import java.math.BigDecimal; diff --git a/src/main/java/com/bookshop/creational/factorymethod/PaymentProcessor.java.md b/src/main/java/com/bookshop/creational/simplefactory/PaymentProcessor.java.md similarity index 100% rename from src/main/java/com/bookshop/creational/factorymethod/PaymentProcessor.java.md rename to src/main/java/com/bookshop/creational/simplefactory/PaymentProcessor.java.md diff --git a/src/main/java/com/bookshop/creational/factorymethod/PaymentProcessors.java b/src/main/java/com/bookshop/creational/simplefactory/PaymentProcessors.java similarity index 91% rename from src/main/java/com/bookshop/creational/factorymethod/PaymentProcessors.java rename to src/main/java/com/bookshop/creational/simplefactory/PaymentProcessors.java index a5d90d8..38b9bfc 100644 --- a/src/main/java/com/bookshop/creational/factorymethod/PaymentProcessors.java +++ b/src/main/java/com/bookshop/creational/simplefactory/PaymentProcessors.java @@ -1,9 +1,9 @@ -package com.bookshop.creational.factorymethod; +package com.bookshop.creational.simplefactory; import java.math.BigDecimal; /** - * The factory method: the only place in the codebase that knows which concrete + * The simple factory: the only place in the codebase that knows which concrete * processor class backs each {@link PaymentMethod}. */ public final class PaymentProcessors { diff --git a/src/main/java/com/bookshop/creational/factorymethod/PaymentProcessors.java.md b/src/main/java/com/bookshop/creational/simplefactory/PaymentProcessors.java.md similarity index 100% rename from src/main/java/com/bookshop/creational/factorymethod/PaymentProcessors.java.md rename to src/main/java/com/bookshop/creational/simplefactory/PaymentProcessors.java.md diff --git a/src/main/java/com/bookshop/creational/factorymethod/PaymentReceipt.java b/src/main/java/com/bookshop/creational/simplefactory/PaymentReceipt.java similarity index 79% rename from src/main/java/com/bookshop/creational/factorymethod/PaymentReceipt.java rename to src/main/java/com/bookshop/creational/simplefactory/PaymentReceipt.java index fb52762..56c6cca 100644 --- a/src/main/java/com/bookshop/creational/factorymethod/PaymentReceipt.java +++ b/src/main/java/com/bookshop/creational/simplefactory/PaymentReceipt.java @@ -1,4 +1,4 @@ -package com.bookshop.creational.factorymethod; +package com.bookshop.creational.simplefactory; import java.math.BigDecimal; diff --git a/src/main/java/com/bookshop/creational/factorymethod/PaymentReceipt.java.md b/src/main/java/com/bookshop/creational/simplefactory/PaymentReceipt.java.md similarity index 100% rename from src/main/java/com/bookshop/creational/factorymethod/PaymentReceipt.java.md rename to src/main/java/com/bookshop/creational/simplefactory/PaymentReceipt.java.md diff --git a/src/main/java/com/bookshop/creational/factorymethod/README.md b/src/main/java/com/bookshop/creational/simplefactory/README.md similarity index 52% rename from src/main/java/com/bookshop/creational/factorymethod/README.md rename to src/main/java/com/bookshop/creational/simplefactory/README.md index c069b58..1f1101d 100644 --- a/src/main/java/com/bookshop/creational/factorymethod/README.md +++ b/src/main/java/com/bookshop/creational/simplefactory/README.md @@ -1,4 +1,4 @@ -# Factory Method +# Simple Factory (Static Factory) ## The problem @@ -8,7 +8,7 @@ about every concrete class -- and adding a payment method means hunting down all ## The pattern -Creation is centralised behind one factory method. Callers name *what* they want +Creation is centralised behind one static factory method. Callers name *what* they want (a `PaymentMethod`), not *how* it's built: ```java @@ -17,22 +17,33 @@ PaymentReceipt receipt = processor.process(total); ``` The concrete classes are `private` nested types -- clients *cannot* couple to them even if they try. -This is the shape the pattern almost always takes in modern Java: a static factory method returning -an interface. (The GoF "subclass overrides the creator" variant survives mostly inside frameworks.) + +## Not the GoF "Factory Method" + +This shape is often loosely called "Factory Method", but the Gang of Four pattern of that name is +something more specific: an abstract *creator* class whose **subclasses override** a method to +decide which product to build. What this package shows -- one static method with a `switch` -- is +the **simple factory** (Effective Java's "static factory method", Item 1), and it's what working +Java code almost always uses. The true GoF shape survives mostly inside frameworks, where the +framework owns the algorithm and your subclass supplies the objects (e.g. overriding a +`createXyz()` hook). If an interviewer or a design review says "Factory Method", it's worth +checking which of the two they mean. ## Benefits - **Decoupling** -- checkout depends only on the `PaymentProcessor` interface; concrete classes can be renamed, replaced, or rewritten freely. -- **One place to change** -- supporting a new payment method touches the factory and nothing else; - the exhaustive `switch` over the enum makes the compiler flag any case you forget. +- **One package to change** -- supporting a new payment method means adding an enum constant and a + `switch` case, both in this package; because the `switch` over the enum is exhaustive, the + compiler flags every spot you forget. Callers don't change at all. - **Hidden implementation choice** -- the factory may return a new instance, a cached one, or a subtype; callers can't tell and don't care. ## Seen in the wild `List.of()`, `Optional.of()`, `Files.newBufferedReader()`, JDBC's `DriverManager.getConnection()`, -`Executors.newFixedThreadPool()`, Spring's `FactoryBean`. +`Executors.newFixedThreadPool()`. Spring's `FactoryBean` and servlet/framework `createXyz()` hooks +are where the GoF subclass-driven variant still earns its keep. ## Implementation @@ -44,4 +55,4 @@ an interface. (The GoF "subclass overrides the creator" variant survives mostly ## Example test -[`PaymentProcessorsTest`](../../../../../../test/java/com/bookshop/creational/factorymethod/PaymentProcessorsTest.java.md) +[`PaymentProcessorsTest`](../../../../../../test/java/com/bookshop/creational/simplefactory/PaymentProcessorsTest.java.md) diff --git a/src/main/java/com/bookshop/solid/ocp/ExpressShipping.java b/src/main/java/com/bookshop/solid/ocp/ExpressShipping.java index 12308c5..eb637fc 100644 --- a/src/main/java/com/bookshop/solid/ocp/ExpressShipping.java +++ b/src/main/java/com/bookshop/solid/ocp/ExpressShipping.java @@ -2,9 +2,11 @@ import java.math.BigDecimal; -/** Next-day courier: flat 7.50. */ +/** Next-day courier: flat rate, never free. */ public class ExpressShipping implements ShippingRate { + static final BigDecimal EXPRESS_COST = new BigDecimal("7.50"); + @Override public String name() { return "express"; @@ -12,6 +14,6 @@ public String name() { @Override public BigDecimal costFor(BigDecimal orderTotal) { - return new BigDecimal("7.50"); + return EXPRESS_COST; } } diff --git a/src/main/java/com/bookshop/solid/ocp/StandardShipping.java b/src/main/java/com/bookshop/solid/ocp/StandardShipping.java index 5449ed8..d556e66 100644 --- a/src/main/java/com/bookshop/solid/ocp/StandardShipping.java +++ b/src/main/java/com/bookshop/solid/ocp/StandardShipping.java @@ -2,9 +2,13 @@ import java.math.BigDecimal; -/** Standard post: 3.00, free for orders of 25.00 or more. */ +/** Standard post: flat rate, free above the threshold. */ public class StandardShipping implements ShippingRate { + /** Business rule: "free shipping over GBP 25" -- named so the rule is findable and searchable. */ + static final BigDecimal FREE_SHIPPING_THRESHOLD = new BigDecimal("25.00"); + static final BigDecimal STANDARD_COST = new BigDecimal("3.00"); + @Override public String name() { return "standard"; @@ -12,8 +16,8 @@ public String name() { @Override public BigDecimal costFor(BigDecimal orderTotal) { - return orderTotal.compareTo(new BigDecimal("25.00")) >= 0 + return orderTotal.compareTo(FREE_SHIPPING_THRESHOLD) >= 0 ? BigDecimal.ZERO - : new BigDecimal("3.00"); + : STANDARD_COST; } } diff --git a/src/test/java/com/bookshop/behavioral/chainofresponsibility/OrderValidationChainTest.java b/src/test/java/com/bookshop/behavioral/chainofresponsibility/OrderValidationChainTest.java index d97c6b1..9d2c808 100644 --- a/src/test/java/com/bookshop/behavioral/chainofresponsibility/OrderValidationChainTest.java +++ b/src/test/java/com/bookshop/behavioral/chainofresponsibility/OrderValidationChainTest.java @@ -55,6 +55,18 @@ void chainOrderIsConfigurable() { assertEquals("address", addressFirst.validate(request).rejectedBy()); } + @Test + @DisplayName("relinking the same checks into a shorter chain drops the old tail") + void relinkedShorterChainHasNoStaleTail() { + StockCheck stock = new StockCheck(Set.of(rareBook.isbn())); + OrderCheck.chainOf(List.of(stock, new FraudCheck())); + OrderRequest fraudWouldReject = new OrderRequest(newBob, List.of(rareBook), "1 High Street"); + + OrderCheck stockOnly = OrderCheck.chainOf(List.of(stock)); + + assertTrue(stockOnly.validate(fraudWouldReject).valid()); + } + @Test @DisplayName("each check is a small unit, testable entirely on its own") void checksAreIndependentlyTestable() { diff --git a/src/test/java/com/bookshop/creational/factorymethod/PaymentProcessorsTest.java b/src/test/java/com/bookshop/creational/simplefactory/PaymentProcessorsTest.java similarity index 89% rename from src/test/java/com/bookshop/creational/factorymethod/PaymentProcessorsTest.java rename to src/test/java/com/bookshop/creational/simplefactory/PaymentProcessorsTest.java index 829cbd4..09cdcd9 100644 --- a/src/test/java/com/bookshop/creational/factorymethod/PaymentProcessorsTest.java +++ b/src/test/java/com/bookshop/creational/simplefactory/PaymentProcessorsTest.java @@ -1,4 +1,4 @@ -package com.bookshop.creational.factorymethod; +package com.bookshop.creational.simplefactory; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -7,7 +7,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; -@DisplayName("Factory Method -- checkout never touches a concrete processor class") +@DisplayName("Simple Factory -- checkout never touches a concrete processor class") class PaymentProcessorsTest { @ParameterizedTest(name = "{0} -> handled by the {1} provider") diff --git a/src/test/java/com/bookshop/creational/factorymethod/PaymentProcessorsTest.java.md b/src/test/java/com/bookshop/creational/simplefactory/PaymentProcessorsTest.java.md similarity index 100% rename from src/test/java/com/bookshop/creational/factorymethod/PaymentProcessorsTest.java.md rename to src/test/java/com/bookshop/creational/simplefactory/PaymentProcessorsTest.java.md diff --git a/src/test/java/com/bookshop/structural/facade/CheckoutFacadeTest.java b/src/test/java/com/bookshop/structural/facade/CheckoutFacadeTest.java index 96d6a62..0a3e1e9 100644 --- a/src/test/java/com/bookshop/structural/facade/CheckoutFacadeTest.java +++ b/src/test/java/com/bookshop/structural/facade/CheckoutFacadeTest.java @@ -19,13 +19,26 @@ class CheckoutFacadeTest { new BigDecimal("32.50")); private final Customer alice = new Customer("c-1", "Alice", "alice@example.com", 1); + /** Records every charge so tests can assert what the facade did -- or didn't -- ask for. */ + private static class RecordingPaymentService extends PaymentService { + private int chargesAttempted; + + @Override + public String charge(Customer customer, BigDecimal amount) { + chargesAttempted++; + return super.charge(customer, amount); + } + } + private InventoryService inventory; + private RecordingPaymentService payments; private CheckoutFacade checkout; @BeforeEach void setUp() { inventory = new InventoryService(); - checkout = new CheckoutFacade(inventory, new PaymentService(), new ShippingService()); + payments = new RecordingPaymentService(); + checkout = new CheckoutFacade(inventory, payments, new ShippingService()); } @Test @@ -46,6 +59,8 @@ void oneCallDoesEverything() { void outOfStockStopsTheFlowBeforePayment() { assertThrows(IllegalStateException.class, () -> checkout.checkout(alice, List.of(book), "1 High Street")); + + assertEquals(0, payments.chargesAttempted, "payment must never be attempted without stock"); assertEquals(0, inventory.stockLevel(book)); } }