Your answer is already pretty good, and after @janos's refactoring using EnumSet
you would/should probably stop IRL.
But what is an improvement (optimization) depends on your values (loss function). if you resolve to "extract till you drop", you can find a few more refactorings to be made.
- One thing that isn't DRY is
if (has(flags, someFlag)) {do something};
snippets.
- those snippets are not reified (they are not explicit methods/variables etc)
- The flag and the snippet it affects do not have an explicit relation.
- the parameters are assigned.
I started with extracting the bodies of if
statements to static methods of signature Comparator<String> spaceInsensitive(Comparator<String> c)
.
I then converted these methods to constants of type Function<Comparator<String>, Comparator<String>>
.
And moved these constants into the @janos's Mode
enum, so that the I have a 1-to-1 mapping between the option and the transformation it does:
public enum Mode {
REVERSED(c -> c.reversed()),
CASE_INSENSITIVE(c -> (s1, s2) -> c.compare(
s1.toLowerCase(Locale.ROOT),
s2.toLowerCase(Locale.ROOT))),
SPACE_INSENSITIVE(c -> (s1, s2) -> c.compare(
s1.replaceAll("\\s", ""),
s2.replaceAll("\\s", "")));
public final Function<Comparator<String>, Comparator<String>> transform;
Mode(Function<Comparator<String>, Comparator<String>> transform) {
this.transform = transform;
}
}
Extracting the repetitive parts of CASE_INSENSITIVE
and SPACE_INSENSITIVE
, we get, (becoming a Three-Arrow-Java-Programmer as a side effect) :
public enum Mode {
REVERSED(c -> c.reversed()),
CASE_INSENSITIVE(transformingEachParam.apply(s -> s.toLowerCase(Locale.ROOT))),
SPACE_INSENSITIVE(transformingEachParam.apply(s -> s.replaceAll("\\s", "")));
public final Function<Comparator<String>, Comparator<String>> transform;
Mode(Function<Comparator<String>, Comparator<String>> transform) {
this.transform = transform;
}
}
private static final Function<Function<String, String>, Function<Comparator<String>, Comparator<String>>>
transformingEachParam = f -> c -> (s1, s2) -> c.compare(f.apply(s1), f.apply(s2));
Changing if
s to ternary operators, for considerations of composability:
public static Comparator<String> getComparator(Set<Mode> flags) {
Comparator<String> result = Comparator.naturalOrder();
result = flags.contains(Mode.SPACE_INSENSITIVE)
? Mode.SPACE_INSENSITIVE.transform.apply(result)
: result;
// ... the same for other options ....
return result;
}
It is now trivial to roll these into a for each loop:
public static Comparator<String> getComparator(Set<Mode> flags) {
Comparator<String> result = Comparator.naturalOrder();
for (Mode mode : Mode.values()) {
result = flags.contains(mode)
? mode.transform.apply(result)
: result;
}
return result;
}
This is another good place to stop, but since we are learning Java 8 let's convert this loop to Stream API. Stream API does not want us (It is an opinionated API) to write reductions as "start with this value, keep modifying it in that way". It wants us to use associative and stateless binary operations to combine elements of a stream. Luckily for us function composition is both associative and stateless.
Final form:
public static Comparator<String> getComparator2(Set<Mode> flags) {
Comparator<String> result = flags.stream()
.map(m -> m.transform)
.reduce(Function.identity(), (f1, f2) -> f1.andThen(f2))
.apply(Comparator.<String>naturalOrder());
// to fulfill "return a lambda expression" requirement ;)
return (s1, s2) -> result.compare(s1, s2);
}