have an issue with designing in the class in proper manner

Question

What’s the issue with the following, and how would I implement it better using OO principles?

My app contains a bunch of shape classes which all inherit from Shape - Circle, Rectangle, Triangle, etc. Some of these need to be displayed on the screen, in which case they need to leverage common screen logic, so there's a ScreenShape superclass which contains the common logic, and ScreenCircle, ScreenTriangle child classes.

Answer 1

I would suggest to make an interface Shape which provides a basic blue print about the shape of the geometric shape and all your classes implement the shape interface and create a separate class ScreenShape(or abstract class) which all of your classes will extend, and provide the method for displaying on screen in the ScreenShape class. for example your rectangle class will be somewhat like this

class rectangle extends ScreenShape implements Shape
{

// provide implementation of Shape interace methods.


// over-ride ScreenShape methods

public void draw()
{
// actual logic of drawing the objects on screen

}

}

Answer 2

The problem your manager has with the design is probably that you are leaking concerns into your Object model. You have mixed the concepts of what a shape is with how it is rendered. So now what if you need to use shapes in a plotting program? With your current design you would need to defined a PlottedShape, PlottedCircle, PlottedSquare etc etc.

You should keep your Shape object hierarchy clean, then define a separate class that can handle rendering of shapes to an output device.

Answer 3

The issue that I see is that you cannot inherit from more than one class in Java (you can implement several interfaces though). So you're better off incorporating the functionality of ScreenShape into Shape, assuming ScreenShape has some concrete code in its methods.

Answer 4

About what you posted in response to @DOC:

@DOK the manager is not satisfied...although he gave me a go ahead with the design but still he said you should research on this.

I suppose the reason your manager is not satisfied with the current design is because it's suffering from a code smell called Parallel Inheritance Hierarchies. Basically, it describes what you're experiencing: every time a new subclass is created, you have to create its counterpart subclass in the parallel hierarchy. That would make your design more difficult to change, and thus, maintain.

Current Answers Review

I'd like to comment some things I didn't agree on the current answers. Basically you're suggested to either to:

1. Inherit from a ScreenShape class
2. Implement drawing logic in each subclass of Shape

I can see two issues going with option n° 1:

• It defines Shape as an interface, so you'll have to re-implement it in each subclass (Rectangle, etc.)
• To overcome the previous you could make ScreenShape implement it, because all kind of shapes will inherit from it. This is bad, and @Perception answer explains why.
• Suppose you are asked now to export shapes to an XML File (this is another operation on shapes, just as is displaying them). Following this approach you'll need to implement that behavior on an XMLShape class and inherit from it, but you're already inheriting from ScreenShape. See the problem?

Option n° 2 forces you to bloat your domain model with presentation concerns, again just as @Perception said :)

Some Goals to Achieve

From the previous, we can see that:

• You'll want your classes to change when your domain model changes, but not when the way you display shapes or export them to XML changes.
• That leads you to the Single Responsibility Principle that states that a class should have only one reason to change.
• We found that displaying, as well as exporting to XML (in the example I gave you) are operations you'll do on shapes, and you'll want to easily add new operations later without changing your shape classes.

Suggested Solution

First of all, get your Shape hierarchy cleaned out, and leave only things that belongs to your problem domain.

Then, you need different ways to display shapes without implementing that behavior in the Shape hierarchy. If you won't be dealing with nested shapes (shapes that contain other shapes), you can make use of a technique called Double Dispatch. It lets you dispatch a method call based on the receiver's type by encoding that information in the method name dispatched to the argument it receives like this:

public class Circle extends Shape {
    public void DisplayOn(IShapeDisplay aDisplay) {
        aDisplay.DisplayCircle(this);
    }
}

public class Rectangle extends Shape {
    public void DisplayOn(IShapeDisplay aDisplay) {
        aDisplay.DisplayRectangle(this);
    }
}

interface IShapeDisplay {
    void DisplayCircle(Circle aCircle);
    void DisplayRectangle(Rectangle aRectangle);
}

Classes implementing IShapeDisplay would be responsible to display each kind of shape. The nice thing about that is you managed to clean out those pesky details from the Shape hierarchy, encapsulating them in their own class. Because of that, now you're able to change that class without modifying Shape subclasses.

Final Comments

You can read more about code smells and Parallel Inheritance Hierarchies in Martin Fowler's book: Refactoring: Improving the Design of Existing Code.

Also, you'd like to check the Design Patterns: Elements of Reusable Object-Oriented Software book if you need dealing with shape nesting: the Composite and Visitor patterns.

Hope it was helpful to you!

Answer 5

Work it from most generic to most specific.

So it should be like:

. Shape (containing code that exists for any kind of 'Shape' - maybe an abstract class or an interface)

.. ScreenShape (containing logic for drawing on the screen)

... Circle (containing logic for drawing a circle on a screen)

... Square (containing logic for drawing a square on a screen)

So:

public abstract class Shape {
  // ... generic Shape stuff here, possibly an interface
  public abstract void getCoordinates();
}

public abstract class ScreenShape extends Shape {
  public void drawOnScreen() {
    // logic for drawing on a screen here
    // likely invoking 'getCoordinates()' 
  }
}

public class Circle extends ScreenShape {
  public void getCoordinates() {
    // circle specific stuff here, implementation of stuff inherited from Shape
  }
  // also inherits whatever 'drawOnScreen()' implementation 
  // is provided in ScreenShape
}

Answer 6

You could implement the draw() method that would implement common logic.

public abstract class Shape{

    public void draw(){
        //common logic here

        drawImpl();

        //more logic here if needed
    }

    public abstract void drawImpl();        

}

Then your implementations would each implement the drawImpl class.

Alternatively you could implement a class ScreenBehaviour and use composition. Then each of your implementations could use a different ScreenBehaviour implementation like this:

public abstract class Shape{
    private ScreenBehaviour screenBehaviour;

    public final void draw(){
        screenBehaviour.execute();
    }
}