2
\$\begingroup\$

I plan to use this code as not a learning demo, but a production-ready (all names / comments to be modified of course) code snippet in my work and add it to my snippet database. I would like to verify that it has no mistakes, silly things and maybe lacking something important.

package view;

 //Component interface
public interface Car {

    void drive();

    void stop();
}

//Basic Component (implements component iface, provides basic functionality
//additional functionality will be added by concrete decorator classes).
//Alternative Names: SimpleCar, BasicComponent, MainComponent
public class BasicCar implements Car {

    @Override
    public void drive() {
        System.out.println("Basic Car moving/rolling!");
    }

    @Override
    public void stop() {
        // NOP
        // real Car surely must implement stop() :))
    }

}

//(abstract) Decorator class. 
//Adds nothing to interface method.
//will be used as template for all concrete decorator classes
//which would 
//1. call ctor of this abstract class
//2. methods overriding drive would call super.drive() + add their functionality
//before or after calling super.drive() - thus modifying.
public abstract class CarDecorator implements Car {
    protected final Car carToDecorate; // carBeforeThisDecorator

    public CarDecorator(Car car) {
        this.carToDecorate = car;
    }

    @Override
    public void drive() {
        carToDecorate.drive();
    }

    @Override
    public void stop() {
        carToDecorate.stop();
    }

}

//concrete Decorator 1.
//Alternative Name: LoudHonkingCar, LoudHonkFeaturedCar, LoudHonkCar,
//WithLoudHonkDecorator, LoudHonkingCarDecorator, LoudHonkCarDecorator
//(thou "Decorator"  word is superfluous - "extends Decorator" explains it!)
// Usually adds one feature (LoudHonk) which is described by its class name
// this feature may mean overriding one OR MORE methods of Car interface
public class WithLoudHonk extends CarDecorator {

    public WithLoudHonk(Car carToBeDecorated) {
        super(carToBeDecorated);
    }

    // Additional functionality can be harcoded into this method.
    // Much better approach is to use private method for this
    // additional functionality.
    @Override
    public void drive() {
        // also can add functionality BEFORE BasicCar functionality
        // System.out.println("honking loudly BEFORE started to move");
        makeHonkingSoundBeforeDrive();
        super.drive();
        makeHonkingSoundAfterDrive();
        // adding functionality AFTER BasicCar functionality
        // System.out.println("honking loudly AFTER started to move");
    }

    // method contains additional functionality
    private void makeHonkingSoundBeforeDrive() {
        System.out.println("honking loudly BEFORE started to move");
    }

    // method contains additional functionality
    private void makeHonkingSoundAfterDrive() {
        System.out.println("honking loudly AFTER started to move");
    }

    // this class does not add anything to stop() method, but it could!

}

//concrete Decorator 2.
//Alternative Name: SportsFeaturedCar, SportsCar,
//WithSportsFeatureDecorator, SportsCarDecorator
public class WithSportsFeature extends CarDecorator {

    public WithSportsFeature(Car carToBeDecorated) {
        super(carToBeDecorated);
    }

    @Override
    public void stop() {
        makeScreechingSound();
        super.stop();
    }

    private void makeScreechingSound() {
        System.out.println("superfast sports car's wheels screeching to stop!");
    }

    // this class does not add anything to drive() method, but it could!

}

public class Driver {

    public static void main(String[] args) {

        // no Decorator so far - just using class BasicCar as ususally.
        Car mySimpleCar = new BasicCar();
        mySimpleCar.drive();
        mySimpleCar.stop();

        System.out.println("***********************");

        // Now using Decorator pattern.
        // Some people use abstract Decorator class instead Component iface:
        // CarDecorator myCarWithAddedFeatures;
        Car myCarWithAddedFeatures;
        myCarWithAddedFeatures = new WithSportsFeature(
                new WithLoudHonk(mySimpleCar));

        myCarWithAddedFeatures.drive();
        myCarWithAddedFeatures.stop();

        // some construct Decorated car not as one-liner, but step-by-step:
        Car myCarWithLoudHonk = new WithLoudHonk(mySimpleCar);
        Car myCarWithLoudHonkAndSportsFeature = new WithSportsFeature(
                myCarWithLoudHonk);

        System.out.println("***********************");

        myCarWithLoudHonkAndSportsFeature.drive();
        myCarWithLoudHonkAndSportsFeature.stop();
    }

}

Just thoght about following: Maybe abstract CarDecorator class shall be made package-private and be put in separate package (than client class - Driver.java), so that Client (Driver class) has no access to abstract CarDecorator ? So that client code can use only concrete decorator classes (WithLoudHonk, ...) so that Client cannot make any conrete decorators himself? But I think this a stupid idea (no reason to disallow doing this to client)...

I will be summarizing all answers here, to later rework my sample honoring them all:

  1. WithLoudHonk -> LoudHonkingCar (rename class, use word Car in
    name, try avoid pronouns ("with")
  2. WithSportsFeature -> SportsFeaturedCar (rename class, same)
  3. rename methods: makeHonkingSoundBeforeDrive, makeHonkingSoundAfterDrive -> honkBeforeDrive, honkAfterDrive. Just honkBefore(), honkAfter() is not clear BEFORE/AFTER WHAT (drive method meant)? RIGHT? I cannot make method name shorter and clearer, can I?
  4. adding features shall depend on some condition in RT, otherwise I could do it in constructor without decorator - so add to Driver dynamic behavior/logic: "if(condition) { decorate - add features to Basic car, else if (other condition) { add different features})
    1. add code to stop() method.

I do appreciate every contribution. Thank you very much!

\$\endgroup\$
3
  • \$\begingroup\$ your interface having package private methods, is that intentional? \$\endgroup\$ – Imus May 11 '17 at 7:40
  • \$\begingroup\$ @Imus even declared without public keyword, all interface methods are public. \$\endgroup\$ – user23908 May 11 '17 at 9:54
  • \$\begingroup\$ @RafaelOsipov interesting. I did not know that. I would still always write them explicitly as public though. \$\endgroup\$ – Imus May 11 '17 at 22:00
2
\$\begingroup\$

The implementation of the pattern itself looks good. And it's a bit funny to read.

Some thoughts:

  • WithLoudHonk/...: You should clarify, that it is a car. For instance WithSportsFeature could be named SportsCar. I don't see pronouns in class names often. Neither in other naming, to be honest.
  • NOOP in stop: You write, a real car must implement this method, but it's not ensured.
  • makeHonkingSoundBeforeDrive() / makeHonkingSoundAfterDrive() - that's a bit of a repition.

In general, the Decorator pattern is a nice thing, if you don't overuse it. And with overuse I mean endless wrapping of instances and super-calls. To answer your question in the last paragraph: Yes, you should in my opinion hide as much as you can and provide concrete cars with a factory. Just think about debugging the drive method in your last example. That's a lot of jumping around until you exactly know what is happening, and it's only sample code.

Also: You're mentioning that this is a demo and you want to use that pattern in production code. Without knowing what the concrete use case would be, we certainly can't tell you, that the pattern will solve your problems. I somehow have the feeling, it's the wrong choice. I think the decorator's strength is, that you can add behavior, but more important: you can add/change behavior at runtime - and I don't see that in your example code.

Hope that helps...

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.