Ndtyjky

Suppose I have an App with different Scenes. Each Scene has a layout that is build up by a number of ScreenElements, e.g., rectangular boxes that will be drawn to the Canvas. For some Scenes, the same boxes will be used. Therefore, in view of DRY, I want to use inheritance.

What I want to do is to store the ScreenElements in a HashMap (or ultimately an EnumMap) so that I can call on the ScreenElements elsewhere in my code to change their attributes.

Here is what I came up with now...

Here is the basic layout:

Public class BasicLayout {

    private HashMap<String, ScreenElement> screenElements;



    public BasicLayout() {

        screenElements = new HashMap<>();

        screenElements.put("BACKGROUND", new ScreenElement(...));

        screenElements.put("BOXONE", new ScreenElement(...));

        screenElements.put("BOXTWO", new ScreenElement(...));

    }



    public HashMap<String, ScreenElement> getScreenElements() {

        return screenElements;

    }



    public ScreenElement getScreenElement(String elementName) {

        return screenElements.get(elementName);

    }



    public void draw(Canvas canvas) {

        for (ScreenElement screenElement: screenElements) {

            screenElement.draw(canvas);

        }

    }

}

Then another layout could be something like:

Public class OtherLayout extends BasicLayout {      

    private HashMap<String, ScreenElement> screenElements;



    public OtherLayout() {

        screenElements = new HashMap<>(super.getScreenElements);

        screenElements.put("BOXTHREE", new ScreenElement(...));

        screenElements.put("BOXFOUR", new ScreenElement(...));

    }



    @Override

    public HashMap<String, ScreenElement> getScreenElements() {

        return screenElements;

    }



    @Override

    public ScreenElement getScreenElement(String elementName) {

        return screenElements.get(elementName);

    }



    @Override

    public void draw(Canvas canvas) {

        for (ScreenElement screenElement: screenElements) {

            screenElement.draw(canvas);

        }

}

So the thing is that by solving code duplication (I no longer need to add BACKGROUND, BOXONE and BOXTWO in my OtherLayout), I introduce code duplication!

I now need to duplicate the getScreenElements, getScreenElement and draw methods. In this case you need to override them, because if you don't, getScreenElements, for example, always return BACKGROUND, BOXONE and BOXTWO, even when you actually want BACKGROUND, BOXONE, BOXTWO, BOXTHREE and BOXFOUR (for example in the Scene that uses `OtherLayout'.

Hope this makes sense and that someone has an ingenious solution!

To clarify:

• At any time BasicLayout should have BACKGROUND, BOXONE and BOXTWO


• At any time OtherLayout should have BACKGROUND, BOXONE, BOXTWO,
  BOXTHREE and BOXFOUR.

This should work:

public class BasicLayout {

    protected HashMap<String, ScreenElement> screenElements = new HashMap<>();



    public BasicLayout() 

    {

        screenElements.put("BACKGROUND", new ScreenElement(...));

        screenElements.put("BOXONE", new ScreenElement(...));

        screenElements.put("BOXTWO", new ScreenElement(...));

    }



    public HashMap<String, ScreenElement> getScreenElements() {

        return screenElements;

    }



    public ScreenElement getScreenElement(String elementName) {

        return screenElements.get(elementName);

    }



    public void draw(Canvas canvas) {

        for (ScreenElement screenElement: screenElements) {

            screenElement.draw(canvas);

        }

    }

}



public class OtherLayout extends BasicLayout {     

    public OtherLayout() {

        screenElements.put("BOXTHREE", new ScreenElement(...));

        screenElements.put("BOXFOUR", new ScreenElement(...));

    }

}

I don't think solving inheritance is a good choice for avoiding duplication. You always violate Liskov substitution principle is some settle way (in your case not so settle:)). And that leads to far bigger problems down the line.

And the second point inheritance should be used only to share behaviour not data.

I think in your case the delegation/composition is a better fit:

public OtherLayout() {

    screenElements = new HashMap<>();

    screenElements.putAll(new BasicLayout().getScreenElements());

    screenElements.put("BOXTHREE", new ScreenElement());

    screenElements.put("BOXFOUR", new ScreenElement());

}

Because from the looks of it the only thing that is common for the two classes are the elements in the source elements map.

And a more general note. Don't be afraid of duplication as much as the wrong abstraction. The quote from the pioneer of modern computer science (Edsger Dijkstra):

The purpose of abstraction is not to be vague, but to create a new
semantic level in which one can be absolutely precise

Based on the sample code, there only needs to be a single class which takes the elements it will be responsible for in the constructor. For example:

public class Layout {

    private final Map<String, ScreenElement> screenElements;



    public Layout(Map<String, ScreenElement> screenElements) {

        this.screenElements = screenElements;

    }



    // other methods

}

From your example, somewhere in the code base would know to create a BasicLayout or OtherLayout. Refactor that code and extract the map, and pass to the constructor. For example:

 // in some method that knows when to create a `BasicLayout` or `OtherLayout`

 Map<String, ScreenElement> basicScreenElements = new HashMap<>();

 basicScreenElements .put("BACKGROUND", new ScreenElement(...));

 basicScreenElements .put("BOXONE", new ScreenElement(...));

 basicScreenElements .put("BOXTWO", new ScreenElement(...));

 Layout basicLayout = new Layout(basicScreenElements);



// ....

 Map<String, ScreenElement> otherScreenElements = new HashMap<>();

 otherScreenElements .put("BOXTHREE", new ScreenElement(...));

 otherScreenElements .put("BOXFOUR", new ScreenElement(...));

 Layout otherLayout = new Layout(otherScreenElements);