views:

158

answers:

2

I'm designing an application that will allow me to draw some functions on a graphic. Each function will be drawn from a set of points that I will pass to this graphic class.

There are different kinds of points, all inheriting from a MyPoint class. For some kind of points it will be just printing them on the screen as they are, others can be ignored, others added, so there is some kind of logic associated to them that can get complex.

How to actually draw the graphic is not the main issue here. What bothers me is how to make the code logic such that this GraphicMaker class doesn't become the so called God-Object.

It would be easy to make something like this:

class GraphicMaker {
    ArrayList<Point> points = new ArrayList<Point>();

    public void AddPoint(Point point) {
        points.add(point);
    }

    public void DoDrawing() {
        foreach (Point point in points) {
            if (point is PointA) {
                //some logic here
            else if (point is PointXYZ) {
                //...etc
            }
        }
    }
}

How would you do something like this? I have a feeling the correct way would be to put the drawing logic on each Point object (so each child class from Point would know how to draw itself) but two problems arise:

  1. There will be kinds of points that need to know all the other points that exist in the GraphicObject class to know how to draw themselves.
  2. I can make a lot of the methods/properties from the Graphic class public, so that all the points have a reference to the Graphic class and can make all their logic as they want, but isn't that a big price to pay for not wanting to have a God class?
+3  A: 

I would do as you suggested and make each point responsible for drawing itself, passing the array of other points to it:

interface ICanDraw {
    void Draw(ArrayList<Point> allPoints);
}

public abstract class Point : ICanDraw {
    ...
}

public PoniePoint : Point {
    public void Draw(ArrayList<Point> allPoints) {
        // do drawing logic here
    }
}

For your GraphicMaker:

public void DoDrawing() {
    foreach (Point point in points) {
        point.Draw(points);
    }
}

(My Java is a bit rusty, so that might not 100% syntactically correct Java but I think it conveys my suggestion).

Michael Shimmins
Aha! I hadn't thought of having the GraphicObject class pass the list of points as argument to the Point's Draw() method. That seems a very good idea. Although I am still unsure of it's not best just to make all possibly relevant properties public in GraphicObject class, as maybe in the future I will need for my point drawing logic not only the list of all points but some other thing. If I passed the list as opposed to having properties public, I'd have to add new parameters to the point's Draw() methods.
devoured elysium
It depends really how the GraphicsObject works, but I think I'd rather not pass it into the Draw method (I might be wrong, its very contextual and I don't have the full context). If you find that you need a subset of information that GraphicsObject can provide, plus all the Points, I would consider creating a class purely for the purpose of passing to the Draw method that contains a List of Points and the additional stuff from GraphicsObject that you need. This maintains single reposinsibilty and separation of concerns.HOWEVER - if GraphicsObject is concerned with drawing then pass it in.
Michael Shimmins
Do take a look at existing systems for inspiration. In Swing, each component has its own paint() that is passed a Grapics object. The Container/Layout/Component approach allows some components to affect other (contained) components. However, I wouldn't pass the list to the Point's Draw though. I'd add a query to the GraphicsObject that returns a list of other points. Like points with a certain type, within some distance, same color, etc.
Devon_C_Miller
+1  A: 

You are correct that each subclass of Point should have its own drawing method overriding the one in the base Point class.

The drawing method should take a reference to the graphics object, which should have public methods/properties for anything that needs to be used in the point drawing methods, including the list of points if that is one of the things some of the drawing methods need.

Why are you concerned about making public methods on the graphics class? Once the code grows, a few extra visible methods is a lot less confusing than one huge method that does everything.

Tom Clarkson
Yes, I think you are right. Having some public properties/methods is definetively the way to go here.
devoured elysium