Search code examples
javaandroidreadabilitycode-readability

Improving legibility on conditional statement


I am building a HTTP server for my android device.

I am using a lot of IF-ELSE statements to handle differnt requests.

As I will be sharing my code with other people for later use, I will have to make it as legible as possible. Right now, I can't even read my code with ease.

I think the problem comes from using a lot of IF-ELSE statements in one class. For example.

if(purpose.equals("readProfile"){
     .....
}
else if(purpose.equals("writeProfile"){
     .....
}
    ....

I tried classifying them in category and ordered the conditions according to their category. But not a lot of legebility was improved. Then I tried writing short comments infront of each conditions. But this made even more mess.

What can be done to increase legibility for conditional statements?


Solution

  • As Luiggi Mendoza stated, this is a follow up of a previous question...

    If you are using Java 7, you can use a switch-case statement for strings

        //month is a String
        switch (month.toLowerCase()) {
            case "january":
                monthNumber = 1;
                break;
              //partsleft out for sake of brevity ..
            default: 
                monthNumber = 0;
                break;
        }
    

    (excerpt from the Oracle Java Tutorials, referenced above.)

    Refactoring

    However, this huge if-else is just part of the problem. As this seems to be a structure growing over time, I'd recommend a thorough refactoring, and using what seems to me is a Strategy pattern. You should:

    Formulate an interface which covers the boundaries for all the use cases:

    interface MyStrategy {
      void execute(MyInputContext input, MyOutputContext output);
    }
    

    (using a void method with MyInputContext and MyOutputContext are just one approach, this is just an example, but to handle requests that have responses, this makes sense, just like how Servlets work)

    Refactor the content of the big IF-ELSE statement into instances of this interface (these will be the strategies):

    //VERY simplified...
    class ReadProfileStrategy implements MyStrategy {
      void execute(MyInputContext input, MyOutputContext output) {
        //do the stuff that was in the if-else block in the "readProfile" part
      }
    }
    
    //... at the branching part:
    MyInputContext input; //build this here
    MyOutputContext output; //build this here
    
    switch (purpose) {
        case "readProfile":
             // no need to always instantiate this, it should be stateless...
             new ReadProfileStrategy().execute();
             break;
        //... left out for sake of brevity
    }
    

    Refactoring step 2

    If this is done, you can add the string IDs to the interface, and the instances themselves, and get rid of the if-else or switch statement altogether, you could create a Map populated even through an IOC container (like), to be up to date, and completely flexible.

    class ReadProfileStrategy implements MyStrategy {
      String getID() {
          return "readProfile";
      }
    
      void execute(MyInputContext input, MyOutputContext output) {
        //do the stuff that was in the if-else block in the "readProfile" part
      }
    }
    

    In the class when requests are processed

    private final Map<String, MyStrategy> strategyMap; //fill the map using your favorite approach, like using Spring application context, using the getCode() to provide the key of the map
    

    In the processing logic:

    MyStrategy strategy = strategyMap.get(purpose);
    if(strategy!=null) {
        strategy.execute();
    }
    else {
        //handle error here
    }