Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Replacing if/else logic with state/strategy pattern

I have read the previous stack exchanges on replacing conditional logic in Java such as IF/ELSE with State/Strategy patterns but I am not sure whether my case is a proper fit for the replacement. Here are coupled I looked at - Long list of if statements in Java and Converting many 'if else' statements to a cleaner approach

I am essentially writing a file download manager and these are my IF/ELSE constructs:

  1. If the file and it's zip file exist then move zip file to zip file directory and read in file
  2. If the zip file exists then unzip file and move zip file to zip file directory and read in file
  3. If the zip file does not exist then download it from specified URL and then unzip it and read in file and move zip file to specified directory.
  4. If the zip file does not exist in URL then create blank file and write it out to disk .

Essentially as I understand it you put these four conditions as keys in a Hashed map and then values are the "Commands" that need to be issued. However I believe you still need an If/Else to decide what action to be invoked on the key that is given as input. So I fail to see the benefit. Can somebody explain ?

like image 986
gansub Avatar asked Oct 26 '14 08:10

gansub


2 Answers

I think you should use GoF pattern Chain of responsibility. You should introduce two interfaces: 1) Condition where you will check proper condition, e.g. "If the zip file does not exist" and return boolean result - "true" if condition is satisfied, otherwise "else", 2) Execution strategy, that will run action assigned with condition, e.g. "download it from specified URL and then unzip it and read in file and move zip file to specified directory." So, 1st interface will be answer to the question "when", and 2nd - "then". "Condition" implementation and "execution strategy" implementation should be combined into "tuple" (or pair, entry, etc). This "tuple" should be moved to collection in order, that you've described. Then, when you need to handle zip file, you'll iterate over collection, invoking conditions and checking results, if result is "true" then invoking appropriate "execution strategy". Also, condition can be combined with execution strategy and moved into single interface/implementation with two methods. Context, that will describe current state of zip file can be passed between conditions/execution strategies. Hope this helps.

Update. Code example (in Java).

/**
 * All implementations should check proper condition
 */
interface Condition { 

  /**
   * Check if condition is satisfied
   *
   * @param pathToFile path to target file
   * @return 'true' if condition is satisfied, otherwise 'false'
   */
  boolean isSatisfied(String pathToFile); //i've made an assumption that you'll manipulate file path for checking file
}
...
/**
 * Childs will wrap some portion of code (if you'll use language, that supports lambdas/functors, this interface/implementation can be replaced with lambda/functor)
 */
interface Action {

  /**
   * Execute some portion of code
   *
   * @param pathToFile path to target file
   */ 
  void execute(String pathToFile);
}
...
class ZipFileExistsCondition implements Condition {

  @Override
  public boolean isSatisfied(String pathToFile) {
   ... //check if zip file exists
  }
}
...
class ZipFileDoesNotExists implements Condition {
  @Override
  public boolean isSatisfied(String pathToFile) {
   ... //download zip file and move it to some temp directory
   //if file downloaded ok, than return 'true' otherwise 'false'
  }
}
...
class AlwaysSatisfiedCondition implements Condition {
  @Override
  public boolean isSatisfied(String pathToFile) {
   ... //always returns 'true', to run action assigned with this condition
  }
}
...
Collection<Map.Entry<Condition, Action>> steps = Arrays.asList(
 new AbstractMap.ImmutableEntry<Condition, Action>(new ZipFileExistsCondition(), 
 new Action() { /*move zip file to zip file directory and read in file*/ }),
 new ZipFileDoesNotExists(), new Action() { /*download it from specified URL and then unzip it and read in file and move zip file to specified directory*/ },
 new AlwaysSatisfiedCondition(), new Action() { /*create blank file and write it out to disk*/  }
);
...
String pathToFile = ...
...
for(Map.Entry<Condition, Action> step: steps) {
 if(!step.getKey().isSatisfied(pathToFile))
   continue;

 step.getValue().execute(pathToFile); 
}  

Remarks: 1) you can implement 'Condition' as anonymous classes, 2) 'AlwaysSatisfiedCondition' can be singleton, 3) if you're using Java/Groovy/Scala, you can use Guava/Apache Commons 'Predicate' instead of 'Condition', 'Function' or 'Closure' instead of 'Action'.

If you need to exit after first 'satisfied' condition and appropriate action execution then just put 'break'/'return' after action execution.

like image 172
nndru Avatar answered Oct 19 '22 05:10

nndru


Following is the proper way to compile this code. The key point here is that AbstractMap.SimpleImmutableEntry is a single entry. If you wish to add more entries then you need to instantiate the class for each entry.

Collection<Map.Entry<Condition,Action>> steps = Arrays.asList
    (
     (new AbstractMap.SimpleImmutableEntry<Condition,Action>
      (new FileExistsCondition(),
       new Action()
       {
       public void execute(String pathToFile){System.out.println("the path to file is srtm " + pathToFile);}
      }
       )
      ),
     (new AbstractMap.SimpleImmutableEntry<Condition,Action>
      (new ZipFileExistsCondition(),
       new Action()
       {
       public void execute(String pathToFile){System.out.println("the path to file is  " + pathToFile);}
      }
       )
      ),
     (new AbstractMap.SimpleImmutableEntry<Condition,Action>
      (new ZipFileDoesNotExistCondition(),
       new Action()
       {
       public void execute(String pathToFile){System.out.println("the path to file is " + pathToFile);}
      }
      )
      )
     );
like image 28
gansub Avatar answered Oct 19 '22 04:10

gansub