views:

93

answers:

2

Attached is a classic Decorator pattern. My question is how would you modify the below code so that you can wrap zero or one of each topping on to the Pizza

Right now I can have a Pepporini -> Sausage --> Pepporini --> Pizza class driving the total cost up to $10, charging twice for Pepporini.

I don't think I want to use the Chain of Responsibility pattern as order does not matter and not all toppings are used?

Thank you

namespace PizzaDecorator
{
public interface IPizza
{
    double CalculateCost();
}

public class Pizza: IPizza
{
    public Pizza()
    {
    }

    public double CalculateCost()
    {
        return 8.00;
    }

}

public abstract class Topping : IPizza
{
    protected IPizza _pizzaItem;

    public Topping(IPizza pizzaItem)
    {
        this._pizzaItem = pizzaItem;
    }

    public abstract double CalculateCost();

}

public class Pepporini : Topping
{
    public Pepporini(IPizza pizzaItem)
        : base(pizzaItem) 
    {   
    }

    public override  double CalculateCost()
    {
        return this._pizzaItem.CalculateCost() + 0.50;
    }


}

public class Sausage : Topping
{
    public Sausage(IPizza pizzaItem)
        : base(pizzaItem)
    {
    }


    public override double CalculateCost()
    {
        return this._pizzaItem.CalculateCost() + 1.00;
    }
}

public class Onions : Topping
{
    public Onions(IPizza pizzaItem)
        : base(pizzaItem)
    {
    }

    public override double CalculateCost()
    {
        return this._pizzaItem.CalculateCost() + .25;
    }  
}
}
+1  A: 

I would not use decorator pattern for this situation. instead, I'd have pizza hold a set of ITopping:

public interface ITopping {
    double cost();
}

the set will guarantee no duplications. now, to calculate the cost of a pizza you'll add it's base price to the sum over all toppings cost

Asaf David
+1 I was going to suggest the same thing, `Toppings` are not `IPizzas`, and should be referenced by h a Pizza, not the other way around. (It is possible this was simply due to Mike's "hiding of the real application," in which case, we'd need to see the real class hierarchy to help you any further).
BlueRaja - Danny Pflughoeft
What would be the purpose of `ITopping`? I don't really see the need for the above design to sub class `Topping` at all. I think a simple `Topping` class that contains a price/description would suffce.
James
+4  A: 

I would create a Topping class which would have a price and make your Pizza class support multiple toppings. Then just calculate the price based on each topping added e.g.

public interface IPizza
{
    double CalculateCost();
}

public class Pizza : IPizza
{
    private List<Topping> toppings = new List<Topping>();
    private double stdCost;

    public Pizza(double cost)
    {
        // this would be the standard cost of the pizza (before any toppings have been added)
        stdCost = cost;
    }

    public Pizza(IList<Topping> toppings)
    {
        this.toppings.AddRange(toppings);
    }

    public void AddTopping(Topping topping)
    {
        this.toppings.Add(topping);
    }

    public void RemoveTopping(Topping topping)
    {
        this.toppings.Remove(topping);
    }

    public double CalculateCost()
    {
        var total = stdCost;
        foreach (var t in toppings)
        {
            total += t.Price;
        }
    }
}

public class Topping
{
    public Topping(string description, double price)
    {
        Description = description;
        Price = price;
    }

    public double Price { get; private set; }
    public string Description { get; private set; }
}

Usage

IPizza p = new Pizza(5.00);
p.AddTopping(new Topping("Pepperoni", 0.50));
p.AddTopping(new Topping("Sausage", 0.50));
var charge = p.CalculateCost(); // charge = 6.00
James
This was what I would suggest. Very logical to me in the Pizza context.
Robb
Mike stated he "hid the *real* application," so presumably he actually needs separate, more complex logic for each "topping" than just a price.
BlueRaja - Danny Pflughoeft
@BlueRaja: I think what Mike was meaning from *hid the real application* he was referring to not supplying a real code sample. He was using Pizza/Topping relationship instead.
James