Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Getting rid of if/else while calling similar classes Java

I have the problem that I want to and need to get rid of some if else cases. I got the following code in my project:

if (ar[4].equals("week")) {

    WeekThreshold wt = new WeekThreshold();
    firstTime = unparsedDate.format(wt.getStartDate().getTime());
    secondTime = unparsedDate.format(wt.getEndDate().getTime());

} else if (ar[4].equals("month")) {

    MonthThreshold mt = new MonthThreshold();
    firstTime = unparsedDate.format(mt.getStartDate().getTime());
    secondTime = unparsedDate.format(mt.getEndDate().getTime());

} else if (ar[4].equals("quarter")) {

    quarterThreshold();

} else if (ar[4].equals("year")) {

    YearThreshold yt = new YearThreshold();
    firstTime = unparsedDate.format(yt.getStartDate().getTime());
    secondTime = unparsedDate.format(yt.getEndDate().getTime());
}

That three classes WeekThreshold, MonthThreshold and YearThreshold extend from an AbstractThreshold class where they get dates from a calendar, but that is not important. The method quarterThreshold() is special and can stay there. But how can I get rid of that if else blocks and have one statement to call different classes?

EDIT: Forgot to mention, the classes that need to be called are from a variety of the array ar[]. If the array ar[4] is month, MonthThreshold must be called, etc.

like image 786
wg15music Avatar asked Aug 12 '15 12:08

wg15music


4 Answers

Multiple possibilities... Do the XYZThreshold classes have a common interface, like Threshold? Then you could assign a variable with that, for example...

Threshold threshold = null;
if ((ar[4].equals("week")) {
  threshold = new WeekThreshold();
} else ... {

}

firstTime = unparsedDate.format(threshold.getStartDate().getTime());
secondTime = unparsedDate.format(threshold.getEndDate().getTime());

That would be a first step. If you wanted, you could, for example, use an enum to store your Thresholds:

enum Thresholds {
  WEEK("week") {

     public Threshold getThreshold() {
           return new WeekThreshold();
     }
  },
  etc.

  private String period;

  private Thresholds(String period) {
    this.period = period;
  }

  public abstract Threshold getThreshold();

  //  ...add a static class to iterate and search by period, 
  // ...so you can write Threshold threshold = Thresholds.getByPeriod("week").getThreshold();
}

Using enums is a personal taste, of course, you can do the same thing with normal classes or by simply putting your if-block for the Threshold-choosing into a seperate class.

like image 163
Florian Schaetz Avatar answered Nov 12 '22 05:11

Florian Schaetz


You can merge the common code (unparsedDate.format(...)) outside like this:

AbstractThreshold at = null;
switch(ar[4]) {
case "week":
    at = new WeekThreshold();
    break;
case "month":
    at = new MonthThreshold();
    break;
case "year":
    at = new YearThreshold();
    break;
case "quarter":
    quarterThreshold();
    break;
}
if(at != null) {
    firstTime = unparsedDate.format(at.getStartDate().getTime());
    secondTime = unparsedDate.format(at.getEndDate().getTime());
}

Of course an overengineered version is possible. Here's just an illustration how it can be implemented using the Java-8 features:

// Map can be initialized only once, then used many times
Map<String, Supplier<AbstractThreshold>> thresholdSuppliers = new HashMap<>();
thresholdSuppliers.put("week", WeekThreshold::new);
thresholdSuppliers.put("month", MonthThreshold::new);
thresholdSuppliers.put("year", YearThreshold::new);

AbstractThreshold at = thresholdSuppliers.getOrDefault(ar[4], () -> null).get();
if(at != null) {
    firstTime = unparsedDate.format(at.getStartDate().getTime());
    secondTime = unparsedDate.format(at.getEndDate().getTime());
} else if(ar[4].equals("quarter"))
    quarterThreshold();
}
like image 29
Tagir Valeev Avatar answered Nov 12 '22 05:11

Tagir Valeev


Here you can make good use of the FactoryPattern

class ThresholdFactory
{
  public static AbstractThreshold getThreshold(String criteria)
  {
    if ( criteria.equals("week") )
      return new WeekThreshold();
    if ( criteria.equals("month") )
      return new MonthThreshold();
    if ( criteria.equals("year") )
      return new YearThreshold();

    return null;
  }
}

The rest of the code looks then like this:

AbstractThreshold at = ThresholdFactory.getThreshold(ar[4]);
if(at != null){
  firstTime = unparsedDate.format(at.getStartDate().getTime());
  secondTime = unparsedDate.format(at.getEndDate().getTime());
} else {
   quarterThreshold();
}
like image 2
BadK Avatar answered Nov 12 '22 03:11

BadK


first create threshold factory,

static enum ThresholdsFactory {


        week(new WeekThreshold()), month(new MonthThreshold())/* etc */;

        static private Map<String,ThresholdsFactory> lookup = new HashMap<String, ThresholdsFactory>();
        static{
            for(ThresholdsFactory val :  ThresholdsFactory.values()){
            lookup.put(val.name(), val);
            }
        }

        public AbstractThreshold threshold;

        public static ThresholdsFactory find(String name){
            return lookup.get(name);
        }

        ThresholdsFactory(AbstractThreshold th) {
            threshold = th;

} }

now all what you need to do is

AbstractThreshold th = ThresholdsFactory.find(ar[4]);

if (th!=null){
    firstTime = unparsedDate.format(th.getStartDate().getTime());
    secondTime = unparsedDate.format(th.getEndDate().getTime());
}
like image 2
user902383 Avatar answered Nov 12 '22 03:11

user902383