views:

162

answers:

4

I have an abstract class, Shape, and I have a Canvas object that the Shape uses to set its position. I need to make sure that all Shapes have a Canvas object, preferably a Canvas object that is the same one across all shapes.

I have thought of a couple of options:

  • Add a canvas parameter to the Shape constructors (there are many)

  • Having some sort of place where the abstract Shape gets its Canvas (when a default is required)

  • Make all my shape construction go through a factory of some sort.

Which one is the best?

I am using C# / .NET 3.5

A: 

What about having ICanvas public property that gets injected by di framework? If you really have a lot of classes implementing Shape, that means a lot of constructors with Canvas parameter, for property you declare it once in Shape and use it everywhere.

grega g
A: 

Have you thought the other way around. I mean having a canvas object and then adding shapes object to it. This way all your shapes objects have a common reference point which is Canvas. A canvas object having a collection of shapes.. This way you can perform common operations to shapes such as resize all the shapes when canvas resizes.

S M Kamran
A: 

For Dependency Injection (DI) the best default pattern is Constructor Injection (your first option) because it can easily be implemented to ensure your Shapes' invariants.

Just add a Guard Clause to your Shape ctor like this:

private readonly Canvas canvas;

protected Shape(Canvas canvas)
{
    if(canvas == null)
    {
        throw new ArgumentNullException("canvas");
    }
    this.canvas = canvas;
}

This ensures that the canvas field is always available and never null.

You can achieve this property in other ways, but Constructo Injection is by far the simplest way to ensure invariants, so I always choose that as my default DI strategy unless I have other needs.

Mark Seemann
A: 

I had a similar idea as S M Kamran: You can have a Canvas property in your Shape class to indicate the canvas to which that shape belongs to, and a Shapes collection in your Canvas class to hold a collection of shapes:

Here's a quick example:

interface ICanvas
{
    // It's a good thing to use an interface
    // in this phase. It will allow you greater
    // freedom later.
}

class Canvas : ICanvas
{
    private Shape.ShapeCollection _shapes;
    public Collection<Shape> Shapes
    {
        get { return _shapes; }
    }

    public Canvas()
    {
        _shapes = new Shape.ShapeCollection(this);
    }
}

class Shape
{
    public class ShapeCollection : Collection<Shape>
    {
        private readonly ICanvas _parent;
        public ShapeCollection(ICanvas parent)
        {
            _parent = parent;
        }

        protected override void InsertItem(int index, Shape item)
        {
            item._canvas = _parent;
            base.InsertItem(index, item);
        }

        protected override void RemoveItem(int index)
        {
            this[index]._canvas = null;
            base.RemoveItem(index);
        }

        protected override void SetItem(int index, Shape item)
        {
            RemoveAt(index);
            InsertItem(index, item);
        }

        protected override void ClearItems()
        {
            while (this.Count != 0)
                this.RemoveAt(this.Count - 1);
        }
    }

    private ICanvas _canvas;
    public ICanvas Canvas
    {
        get { return _canvas; }
    }
}

By adding a shape to a canvas, Shape.Canvas property will be updated automatically:

Canvas canvas = new Canvas();
Shape circle = new Shape();

canvas.Shapes.Add(circle);

Note that ShapeCollection is a nested class inside Shape, because it's the only way to set the private _canvas property.

Generic collections (like Collection<Shape>) are often used for similar problems, because you can override their InsertItem/RemoveItem properties to add custom functionality when the collection is being changed.

You can also create an "Empty/Default Canvas" singleton, and use it instead of null when shape is not assigned to a real canvas (just to avoid checking if Shape.Canvas is null every time).

Groo