7
\$\begingroup\$

Is this a good use of the decorator pattern? If not, why not? Is this a good idea to use for example when we have objects communicating with MVC through PHP or via ASP? To send data through the model to a decorator class that extends so ; and also using this data to extend to other classes.

For example, in theory one can create a database and relation it towards a model class this model class is then created through a model class but decorated with a model class that extends its base constructor. Having the ability to get methods from the CarPinto object using the constructor injection. Is this a practical way of implementing it via a decorator or simply a means of decorating data on the fly?

Program Class

class Program
{
    static void Main(string[] args)
    {
        Car lambo = new CarLamborghini(new Decorator(new LuxuryCar()));
        Car cheapyCar = new CarPinto(new Decorator(new BummyCar()));
        Console.ReadLine();
    }
}

Here is the Decorator Class

class Decorator : Car
{
    Car car;

    public Decorator(Car _car) {
        this.car = _car;
    }

    public override void drive()
    {
        car.drive();
    }

    public override void stop()
    {
        car.stop();
    }

    public override void park()
    {
        car.park();
    }
}

Here are the two classes derived from Car

class LuxuryCar : Car
{
    public override void drive()
    {
        Console.WriteLine("I'm driving a fast car");
    }
    public override void stop()
    {
        Console.WriteLine("I have come to a complete stop quick");
    }

    public override void park()
    {
        Console.WriteLine("I am in park in a close range");
    }
}


class BummyCar : Car
{
    public override void drive()
    {
        Console.WriteLine("I am a slow driver");
    }

    public override void stop()
    {
        Console.WriteLine("I have stopped abrubtly");
    }

    public override void park()
    {
        Console.WriteLine("I am in park :{");
    }
}

And here is our decorator classes that inherits from decorator

class CarLamborghini : Decorator
{
    public CarLamborghini(Decorator decorator) : base(decorator) {
        decorator.drive();
        decorator.stop();
        decorator.park();
    }
}

And the other

class CarPinto : Decorator
{
    public CarPinto(Decorator decorator): base(decorator) {
        decorator.drive();
        decorator.stop();
        decorator.park();
    }
}

My question is this a good way to use the decorator pattern and its implementation using abstract classes instead of interfaces

\$\endgroup\$
  • 5
    \$\begingroup\$ What problem are you solving with the decorator? \$\endgroup\$ – Johan Larsson Sep 20 '15 at 18:14
  • \$\begingroup\$ being able to get a Car and return it within the class Program with DI \$\endgroup\$ – noob1992 Sep 20 '15 at 18:16
  • \$\begingroup\$ If I needed to get a car from the decorator patterns concrete class I can allow so by making a new instance and programming the logic inside the program correct ? therefore allowing me access to make as many concrete classes from the constructor to the program where I can add logic to instantiate objects of class Car in theory \$\endgroup\$ – noob1992 Sep 20 '15 at 18:17
  • 2
    \$\begingroup\$ An interface will give more flexibility. The decorator pattern is all about adding functionality to an object without letting the object know about it. \$\endgroup\$ – Harshdeep Mehta Sep 20 '15 at 19:04
  • \$\begingroup\$ FYI, I've rolled back your edit as it invalidates advice given in existing answer. \$\endgroup\$ – Ethan Bierlein Sep 21 '15 at 0:32
18
\$\begingroup\$

Composition vs. Inheritance, interface vs. abstract class

You are mixing up concepts here - there's too much inheritance going on, and this pattern is completely about composition. I'm sure you've seen this before:

Favor composition over inheritance.

You have an abstract base class with either a bunch of abstract methods, or a bunch of virtual methods with "sensible defaults":

public abstract class Car
{
    public virtual void Drive()
    {
        Console.WriteLine("♪On the road again♪");
    }

    public virtual void Stop()
    {
        Console.WriteLine("BRAAAAAAAAAKE!");
    }

    public virtual void Park()
    {
        Console.WriteLine("Your destination is on the right.")
    }
}

Given you override all these methods in all implementations you've shown, I'm going to guess the Car class looks something more like this then:

public abstract class Car
{
    public abstract void Drive();
    public abstract void Stop();
    public abstract void Park();
}

Nice abstraction. But there's a more lightweight, more flexible way to do this:

public interface ICar
{
    void Drive();
    void Stop();
    void Park();
}

Interfaces make the pattern easier to pick up I find. If I had to describe the Decorator Pattern in one sentence:

The decorator implements the same interface as the type it decorates, each method calling the decorated type's members, adding functionality as needed.

Which makes this:

class CarLamborghini : Decorator

...quite confusing.

This seems rather convoluted:

Car lambo = new CarLamborghini(new Decorator(new LuxuryCar()));

The decorator

The Decorator class serves absolutely no purpose in your design. It's a no-op. Sad, because it's pretty close to how I'd write one:

class Decorator : ICar
{
    private readonly ICar _car;

    public Decorator(ICar car) {
        _car = car;
    }

    public void Drive()
    {
        _car.Drive();
    }

    public void Stop()
    {
        _car.Stop();
    }

    public void Park()
    {
        _car.Park();
    }
}

Notice:

  • PascalCase member names
  • private readonly _camelCase fields
  • camelCase parameters
  • Decorated type is an interface

But it's a no-op... there's no point in having such a decorator! A decorator should have a purpose - and a name that tells us what that purpose is. Take one that makes a car go "Squeeeeeeak!" when you Stop it:

class SqueakyBrakesDecorator : ICar
{
    private readonly ICar _car;

    public SqueakyBrakesDecorator(ICar car) {
        _car = car;
    }

    public void Drive()
    {
        _car.Drive();
    }

    public void Stop()
    {
        Console.WriteLine("Squeeeeeeak!");
        _car.Stop();
    }

    public void Park()
    {
        _car.Park();
    }
}

Test drive

Now if you want a squeaking Lamborghini, you can have one! And because you're coding against abstractions, the caller will not know that their flashy sports car has such a defect!

public static class CarTester
{
    public static void TestDrive(ICar car)
    {
        car.Drive();
        car.Stop();
        car.Park();
    }
}

Notice how this code is completely oblivious of anything like a decorator? As far as it knows, it's getting an ICar: how each method is actually implemented isn't its concern, all that matters is that the concrete type implements the ICar interface.

And that's exactly what a decorator does: it implements the interface of the type it's decorating:

class SqueakyBrakesDecorator : ICar
{
    private readonly ICar _car;

    public SqueakyBrakesDecorator(ICar car) {
        _car = car;
    }

So now we can run CarTester.TestDrive and give it a SqueakyBrakesDecorator and it won't even blink - and that is the beauty of this pattern.

Look at the Stop method of the SqueakyBrakesDecorator above:

    public void Stop()
    {
        Console.WriteLine("Squeeeeeeak!");
        _car.Stop();
    }

Every method is calling _car.MethodWeAreIn: that's how a decorator can work, that's how their actions can be "piled up" - by decorating a decorator, you get cumulative action, because they all call each other.

So a decorator gets to decide what code runs before or after the decorated code; it also gets to decide what parameters are going in (derived or not from the parameters it received), when parameters are involved.


Beyond the car: real-life decorators

The decorator pattern is pretty cool in the real world - I find the Car example doesn't serve it very well.

Take logging as a concern, in an application that uses an IFileWriter to write to the file system, for example:

public class LoggingFileWriter : IFileWriter
{
    private readonly IFileWriter _writer;
    private readonly ILogger _logger;

    public LoggingFileWriter(IFileWriter writer, ILogger logger)
    {
        _writer = writer;
        _logger = logger;
    }

    public void Write(string text)
    {
        try
        {
            _writer.Write(text);
            _logger.Info("File written: " + _writer.FileName);
            _logger.Debug(text);
        }
        catch (IOException exception)
        {
            _logger.Error(exception);
        }
    }
}

The pattern clearly helps adhering to SRP - the Single Responsibility Principle. It takes the responsibility of logging out of some FileWriter class, and passes it to this LoggingFileWriter class, who's in turn not concerned with the implementation details of how the file actually ends up written.

By calling the decorated object's own methods, you enable this "cumulation" of decorators: you can have a LoggingFileWriter that's taking in a TweetingFileWriter that sends a Tweet when the file contains a <Tweet>Up to 140 characters</Tweet> tag, that's taking in a... well you get the idea: they're all the same interface. And they each have their very own specific functionality, and to add more functionality you just write another decorator and build the functionality on top of what you've already written: this adheres to the Open/Closed Principle, which states that your classes should be open to extension, but closed to modification - a class shouldn't have 20 reasons to be modified; the Decorator Pattern is but a tool to achieve this.

Constructors and constructor injection

The constructor:

public LoggingFileWriter(IFileWriter writer, ILogger logger)

Is documenting that the type depends on abstractions, not concrete types. That's the spirit of Dependency Injection Principle: instead of newing up a specific ILogger implementation, we're getting one constructor-injected.

Constructor injection is the most useful and common type of dependency injection.

Notice what the constructor does:

public LoggingFileWriter(IFileWriter writer, ILogger logger)
{
    _writer = writer;
    _logger = logger;
}

It assigns private readonly fields. Nothing more. Compare to:

public CarLamborghini(Decorator decorator) : base(decorator) {
    decorator.drive();
    decorator.stop();
    decorator.park();
}

Don't do work in constructors. It will bite you.

\$\endgroup\$
10
\$\begingroup\$

You received a very good answer addressing your implementation of the decorator pattern, but I see some issues with the over all design of this code. It seems that your real goal is to create a "standard" and a "sports" car, which could be accomplished by implanting an ICar or inheriting shared functionality from an abstract base Car class. The problem is, real cars don't come in just sports/standard versions. They're composed of different parts. Let's focus on making the car go and stop. This means we'll need two interfaces.

public interface IEngine
{
    void Start();
    void Stop();
    bool IsRunning { get; }
}

public interface IBrake
{
    void Apply();
}

Then Car would look something like this.

public class Car
{
    public Car(IEngine engine, IBrake brakes)
    {
        _engine = engine;
        _brakes = brakes;
    }

    public void Start()
    {
        if (!_engine.IsRunning)
        {
            _engine.Start();
        }
    }

    public void Stop()
    {
        if (_engine.IsRunning)
        {
             _brakes.Apply();
             _engine.Stop();
             return;
         }

         Console.WriteLine("You're already stopped!");
    }

Which leaves us free to implement the brakes and engine however we see fit, so long as they adhere to the contract.

 public class PerformanceBrakes : IBrake
 {
      public void Apply()
      {
           Console.WriteLine("Wow! We stopped fast!");
      }
  }

Now, we can mix and match parts to create whatever kind of Car we like.

var fastJallopy = new Car(new PerformanceEngine(), new SqueakyBrakes());
var sportsCar = new Car(new PerformanceEngine(), new PerformanceBrakes());

This models the real world counter part much more closely and is considerably more flexible.

\$\endgroup\$
  • \$\begingroup\$ Just to be clear, what I've shown here isn't the decorator pattern, it's an entirely different design. \$\endgroup\$ – RubberDuck Sep 21 '15 at 0:31

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.