Adding a feature to legacy code while trying to improve it can be quite challenging, but also quite straightforward. Nothing angers me more (ok, I might be a little exaggerating) than stumbling upon such pattern:
public Foo getFoo(Bar bar) {
if (bar instanceof BarA) {
return new FooA();
} else if (bar instanceof BarB) {
return new FooB();
} else if (bar instanceof BarC) {
return new FooC();
} else if (bar instanceof BarD) {
return new FooD();
}
throw new BarNotFoundException();
}Apply Object-Oriented Programming
The first reflex when writing such thing - yes, please don’t wait for the poor guy coming after you to clean your mess, should be to ask yourself whether applying basic Object-Oriented Programming couldn’t help you. In this case, you would have multiple children classes of:
public interface FooBarFunction<T extends Bar, R extends Foo> extends Function<T, R>For example:
public class FooBarAFunction implements FooBarFunctio<BarA, FooA> {
public FooA apply(BarA bar) {
return new FooA();
}
}|
Not enjoying the benefits of Java 8 is not reason not to use this:
just create your own |
Use a Map
I must admit that it not only scatters tightly related code in multiple files (this is Java…), it’s unfortunately not always possible to easily apply OOP. In that case, it’s quite easy to initialise a map that returns the correct type.
public class FooBarFunction {
private static final Map<Class<Bar>, Foo> MAPPINGS = new HashMap<>();
static {
MAPPINGS.put(BarA.class, new FooA());
MAPPINGS.put(BarB.class, new FooB());
MAPPINGS.put(BarC.class, new FooC());
MAPPINGS.put(BarD.class, new FooD());
}
public Foo getFoo(Bar bar) {
Foo foo = MAPPINGS.get(bar.getClass());
if (foo == null) {
throw new BarNotFoundException();
}
return foo;
}
}Note this is only a basic example, and users of Dependency Injection can easily pass the map in the object constructor instead.
More than a return
The previous Map trick works quite well with return statements but not with code snippets.
In this case, you need to use the map to return an enum and associate it with a switch-case.
public class FooBarFunction {
private enum BarEnum {
A, B, C, D
}
private static final Map<Class<Bar>, BarEnum> MAPPINGS = new HashMap<>();
static {
MAPPINGS.put(BarA.class, BarEnum.A);
MAPPINGS.put(BarB.class, BarEnum.B);
MAPPINGS.put(BarC.class, BarEnum.C);
MAPPINGS.put(BarD.class, BarEnum.D);
}
public Foo getFoo(Bar bar) {
BarEnum barEnum = MAPPINGS.get(bar.getClass());
switch(barEnum) {
case BarEnum.A:
// Do something;
break;
case BarEnum.B:
// Do something;
break;
case BarEnum.C:
// Do something;
break;
case BarEnum.D:
// Do something;
break;
default:
throw new BarNotFoundException();
}
}
}Note that not only I believe that this code is more readable but it’s also a fact it has better performance and the switch is evaluated once as opposite to each if…else being evaluated in sequence until it returns true.
Note it’s expected not to put the code directly in the statement but use dedicated method calls.
Conclusion
I hope that at this point, you’re convinced there are other ways than sequences of if…else.
The rest is in your hands.