HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Decorator for cars

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
decoratorcarsfor

Problem

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 :{");
}
}

Solution

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
{

Code Snippets

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.")
    }
}
public abstract class Car
{
    public abstract void Drive();
    public abstract void Stop();
    public abstract void Park();
}
public interface ICar
{
    void Drive();
    void Stop();
    void Park();
}
class CarLamborghini : Decorator
Car lambo = new CarLamborghini(new Decorator(new LuxuryCar()));

Context

StackExchange Code Review Q#105184, answer score: 18

Revisions (0)

No revisions yet.