-1

How can I improve the design of the code below:

class Foo {
   public Configuration configure() {
      return new Configuration().withPropertyA().withPropertyB();
   }
}

class Configuration{


   private boolean propertyA = false;
   private boolean propertyB = false;

   public Configuration withPropertyA() {
       this.propertyA = true;
       return this;
   }

    public Configuration withPropertyB() {
       this.propertyB = true;
       return this;
   }

   public boolean hasPropertyA() {
      return this.propertyA;
   }

   public boolean hasPropertyB() {
      return this.propertyB;
   }

   ...
}

class Client{

public void someMethod(Configuration config) {
   if (config.hasPropertyA() {
      doStuffA();
   }
   if (config.hasPropertyB() {
      doStuffB();
   }
   //...
}

}

I.e., the Configuration class holds some flags for the caller (Client) that tell to the caller which "things" need to be configured. The Client knows what to do for each of the flags if they are set. I want to get rid of the if statement in Client as well as of the concept of simply setting boolean flags in the Configuration instance. Can you suggest a more "generic" and object oriented approach to this?

kind regards

alexbt
  • 16,415
  • 6
  • 78
  • 87
Moonlit
  • 5,171
  • 14
  • 57
  • 95

3 Answers3

1

You may use the Strategy pattern. Each If becomes a Strategy, which implements doStuff, with its own logic. When a property (A, B...) is set, you add the strategy to the list. You simply need to loop on the strategies and execute them, without any ifs:

public class Foo  {
    public Configuration configure() {
        return new Configuration().withPropertyA().withPropertyB();
    }
}

class Configuration {
    Set<StuffStrategy> strategies = new HashSet<StuffStrategy>();

    public Configuration withPropertyA() {
        strategies.add(new PropertyAStrategy());
        return this;
    }

    public Configuration withPropertyB() {
        strategies.add(new PropertyBStrategy());
        return this;
    }

    public void executeStrategies() {
        for (StuffStrategy strategy : strategies) {
            strategy.doStuff();
        }
    }
}

interface StuffStrategy {
    public void doStuff();
}
class PropertyAStrategy implements StuffStrategy {
    @Override
    public void doStuff() {
    }
}

class PropertyBStrategy implements StuffStrategy {
    @Override
    public void doStuff() {
    }
}

class Client {
    public void someMethod(Configuration config) {
        config.executeStrategies();
    }
}
alexbt
  • 16,415
  • 6
  • 78
  • 87
1

I don't believe that you can find a more OO approach than what you currenly do especially if you can have more parameters, indeed this is somehow the well know design pattern Builder, the best you can do is finishing implementing this pattern properly by making your object Configuration immutable from outside only your builder class should be able to create an instance of Configuration. Your builder should be a mutable static inner class with a build() method that returns an immutable Configuration instance.

Nicolas Filotto
  • 43,537
  • 11
  • 94
  • 122
  • I think @Alex provided a clean object oriented approach for this. However, I'll give you an up-vote for the builder hint. thx – Moonlit Jun 14 '16 at 05:08
  • @user1291235 even if it seems to be "clean" as you said, it is not flexible at all, what will you do if your needs change a little bit and you have to support cases where you have both property A and B or you have A but not B, your code will then be a nightmare to maintain – Nicolas Filotto Jun 14 '16 at 10:19
-2

It looks like you are writing a pitfall

try using in that case something more flexible in the design.... like an enumerator... take a look at this:

Example:

public class Foo {
    public Configuration configure() {
    return new Configuration(Config.A);
    }
}

class Configuration {

enum Config{
A,B, NONE}

    private Config propertyConfig = Config.NONE;

    public Configuration(Config a) {
    propertyConfig=a;
    }


    public Config getConfig() {
    return this.propertyConfig;
    }

    ...
}

class Client {

    public void someMethod(Configuration config) {
       switch (config.getConfig()) {
    case A:
        System.out.println("a config");
        break;
    case B:
        System.out.println("b config");
        break;

    default:
        break;
    }
    // ...
}

}
Community
  • 1
  • 1
ΦXocę 웃 Пepeúpa ツ
  • 47,427
  • 17
  • 69
  • 97
  • 2
    So you replaced `if` statements with a `switch-case`. How is that better / different? – Tunaki Jun 13 '16 at 20:25
  • Are you aware that a switch case if faster than a if else, omitting the fact of the design in the code... – ΦXocę 웃 Пepeúpa ツ Jun 13 '16 at 20:30
  • Perhaps a bit faster although I would want to measure this in a real environment. That said, the question is about the design of the code. – Tunaki Jun 13 '16 at 20:34
  • go take a look at this http://stackoverflow.com/q/4922932/982161, after you get the point please open a chat room and we discuss the topic... – ΦXocę 웃 Пepeúpa ツ Jun 13 '16 at 20:35
  • Then you may have missed the [second answer](http://stackoverflow.com/a/4923029/1743880) (or [this one](http://stackoverflow.com/a/4922961/1743880)) in that linked question. And, still, the question is about improving the global design of the code. – Tunaki Jun 13 '16 at 20:37
  • the perf is out of topic here anyway, the PO asks for a more object oriented approach and I don't believe that a switch statement is real OO approach – Nicolas Filotto Jun 13 '16 at 20:42