Skip to content

Latest commit

 

History

History
61 lines (31 loc) · 3.25 KB

PATTERN_REVIEW.md

File metadata and controls

61 lines (31 loc) · 3.25 KB

Pattern Review Notes

These notes capture follow-up improvements for examples that mostly work, but could express the intended design pattern more clearly.

Structural Patterns

Adapter

File: structural/adaptor/RightHandInLeftHandTraffic.java

Current issue: RightHandInLeftHandTraffic inherits the same Car target interface as the other car classes and overrides behavior. That shows specialization more than adaptation.

Suggestion: Make the adapter wrap an incompatible adaptee instead of only subclassing it. For example, keep Car as the target expected by Main, make JapaneseRightHandCar expose a different API, then make RightHandInLeftHandTrafficAdapter extends Car and delegate translated calls to the wrapped JapaneseRightHandCar.

Decorator

File: structural/decorator/Activity.java

Current issue: Activity extends concrete Commuter and can exist without a wrapped commuter. This weakens the usual Decorator roles.

Suggestion: Introduce a common component abstraction, such as Commutable, with commute() and claimDestination(). Commuter should implement it, and Activity should also implement it while wrapping another Commutable. Concrete activities then decorate any component transparently.

Composite

File: structural/composite/ConcreteMilitaryUnit.java

Current issue: reportUnitCounts() and reportWeaponCounts() mutate unitCounts and weaponCounts while reporting, so repeated calls inflate the result.

Suggestion: Compute counts with local variables each time, or update aggregate counts only when add() and remove() are called. The Composite structure itself is correct.

Creational Patterns

Builder

File: creational/builder/BeverageDirector.java

Current issue: The builders model a production sequence, but they do not build and return a product object.

Suggestion: Add a Beverage or Bottle product class. Each concrete builder should mutate that product during each step and expose getResult(). The director can still enforce the construction order, but the outcome should be an object that differs by builder.

Singleton

File: creational/singleton/EmergencyTelephone.java

Current issue: The implementation uses double-checked locking, but instance is not declared volatile.

Suggestion: Change the field to private static volatile EmergencyTelephone instance;. Alternatively, simplify the example with an enum singleton or the initialization-on-demand holder idiom.

Behavioral Patterns

Template Method

File: behavioural/templatemethod/Housekeeping.java

Current issue: Main controls the algorithm order by calling washClothes() and cleanUp() separately. The base class does not define a single template method.

Suggestion: Add a final method such as final String doHousekeeping() in Housekeeping. That method should call protected primitive operations like washDetails() and cleanUpDetails() in the fixed order. Subclasses should only customize those primitive steps.

Overall Assessment

The pattern choices are mostly correct. The main improvements are about making participant roles clearer: target versus adaptee for Adapter, component versus decorator for Decorator, fixed algorithm versus primitive operations for Template Method, and product construction for Builder.