Is it a good idea to divide every graphic shape in two parts - its geometry and rendering components?

Question

I am writing a simple graphic editor for a university project using C#.
I created a hierarchy where every type of shape - circles, rectangles, ellipses, ... - are inherited from a base class Shape, it contains a shape color as a protected field.

public abstract class Shape {
        protected Color color;

        public Shape(Color c) {
            color = c;
        }

        public Color FigureColor {
            get { return color; }
            protected set { color = value; }
        }

        public abstract void render(Bitmap canvasToDrawOn);

        public abstract void unrender(Bitmap canvasToDrawOn);

        public abstract void renderPrototype(Bitmap canvasToDrawOn);
    }

The idea is that I want every type of shape to encapsulate it`s geometry part, for example:

public class TwoDPoint: TwoDShape {
        Point2DGeometry point;

        public TwoDPoint(Color c, Point2DGeometry p): base(c) {
            point = new Point2DGeometry(p);
        }

public struct Point2DGeometry {
        Int32 x;
        Int32 y;

        public Point2DGeometry(Int32 X, Int32 Y) {
            x = X;
            y = Y;
        }

        public Point2DGeometry(Point2DGeometry rhs) {
            x = rhs.Abscissa;
            y = rhs.Ordinate;
        }

        public Int32 Abscissa {
            get { return x; }
            private set { x = value; }
        }

        public Int32 Ordinate {
            get { return y; }
            private set { y = value; }
        }
    }

Ellipse class will encapsulate EllipseGeometry, etc.
Is it a good decision in terms of OO design, and if it is, may be it is the better idea to create a parallel hierarchy of this geometry-only types?

Answer 1

I think the answer is yes, because it follows along in the track laid down by the venerated MVC pattern.

Another advantage is that it gives you the option of swapping out graphics libraries without having to rewrite all those base geometric classes. Use an interface to make that even easier to do.

Answer 2

I would say that it depends. Are you going to reuse your geometries objects outside of your shape objects, or share geometry objects as Flyweights? If not, you may be complicating your class hierarchy unnecessarily.

Looking over your code you may also want to consider the following:

1. Having some sort of naming convention between your private fields and your public properties. There's no point getting fancy with Abscissa/Ordinate when your private variables are x/y and your constructor takes X/Y. Ditto Color and FigureColor in Shape.

2. In Shape, why make "color" protected, when "FigureColor" also has a protected setter? In most APIs (and you can think of your abstract class as an API to it's derived classes) you'll want to minimize the confusion of which members to use when changing state. In this particular example, I'd most likely make "color" private.

3. Watch for "internal classes" in your class hierarchy which don't add any value. For instance, you have an abstract Shape class, and from the look of it, TwoDShape as well. What's the difference between the two? Do you have support for 3 dimensions? Does your inheritance tree look more like a stick?

HTH, and good luck with your project.