-2

I have code in the following form:

public void drawObject(MyObject myObject) {
  RootElement root = new RootElement();

  if (myObject.hasA()) {
     root.addElement(new XElement());
  } else {
     root.addElement(new YElement());
  }
  if (myObject.hasB()) {
     root.addElement(new ZElement("A"));
  }
  if (myObject.hasC()) {
     root.addElement(new ZElement("B));
  }

  //and so on for around 20 more conditions

  root.draw();
}

I think that's not optimal. However, I can't think of any better solution to this. Is there any design pattern that I could use?

SCM
  • 31
  • 3
    It would be helpful if you could provide more context about your actual problem domain. "Foo-bar" examples seldom provide that context. – Robert Harvey Apr 22 '20 at 15:01
  • As amon said, if this is essential complexity, the only thing you can do is decide where to locate that complexity. In the object, in the collection, etc. Without knowing more about your context (as RobertHarvey notes), one can only present those various options. There is also a strategy pattern that might be another place to relocate the complexity. It seems possible, though, that if we saw a larger version of the problem, maybe the best option would be a substantial change from this approach; who knows? – Erik Eidt Apr 22 '20 at 19:26
  • Does this answer your question? [Choosing the right Design Pattern](https://softwareengineering.stackexchange.com/questions/227868/choosing-the-right-design-pattern) – gnat Apr 23 '20 at 00:19

2 Answers2

2

There isn't always a design pattern that will elegantly solve complexity. Some complexity is inherent in the problem you're solving. At most, you can shuffle the complexity around.

So quite possibly, the If Statement Pattern is the best you can do here.

However, if your MyObject type is some interface and different concrete types have different but simpler logic for drawing themselves, you could replace conditionals with polymorphism. This splits up the complexity and moves it into individual classes. For example:

public void drawObject(MyObject myObject) {
  myObject.toElement().draw()
}

interface MyObject {
  ...
  public RootElement toElement();
}

class MyObjectA implements MyObject {
  ...
  @Override
  public RootElement toElement() {
    RootElement root = new RootElement();
    root.addElement(new XElement());
    return root;
  }
}

class MyObjectB implements MyObject { ... }
class MyObjectC implements MyObject { ... }

Sometimes, instead of testing whether some element should be added, it could make sense to use the Null Object Pattern. You would then always add an element, but the element may not draw anything. Whether that would simplify things depends on your specific code, and your snippet is to abstract to make a statement on this.

But again: maybe this complexity cannot be avoided. As a last resort, you can extract individual conditions into separate helper functions and give them a useful name. This can make a large complex method easier to read, but makes the overall code more complex. Something like this:

public void drawObject(MyObject myObject) {
  RootElement root = new RootElement();

  addElementForA(root, myObject);
  addElementForB(root, myObject);
  addElementForC(root, myObject);
  ...

  root.draw();
}

private static void addElementForA(RootElement root, MyObject myObject) {
  if (myObject.hasA()) {
    root.addElement(new XElement());
  } else {
    root.addElement(new YElement());
  }
}

private static void addElementForB(RootElement root, MyObject myObject) {
  if (myObject.hasB()) {
    root.addElement(new ZElement("A"));
  }
}

private static void addElementForC(RootElement root, MyObject myObject) {
  if (myObject.hasC()) {
    root.addElement(new ZElement("B"));
  }
}

Don't use placeholder names like this in the real code, but think about what the condition represents in your problem domain. E.g. if this would assemble a web page with an optional footer, you might call one of these helpers maybeAddFooter().

amon
  • 132,749
  • 27
  • 279
  • 375
2

This code features many things I'd improve but here's the first refactoring:

public void drawObject(MyObject myObject) {
  RootElement root = new RootElement();
  Element element = myObject.elementFactory();
  root.addElement(element);

  root.draw();
}

Why? Feature Envy, Tell. Don't ask., and Functions Should be Small. As small your naming talent permits.

MyObject knows everything you're asking about to make the element. Why not just tell MyObject to make the element? Now you don't have to know so much about the insides of MyObject. One way to do that is an Abstract Factory.

candied_orange
  • 102,279
  • 24
  • 197
  • 315