1

Say I have a Filter interface

public interface Filter {
   public boolean satisfies(Earthquake earthquake)
}

Which can have different implementations (one such below)

public final class MagnitudeFilter implements Filter {

    private final double minimumMagnitude;
    private final double maximumMagnitude;
    private final ComparisonStrategy strategy;

    public MagnitudeFilter(double minimumMagnitude, double maximumMagnitude, ComparisonStrategy strategy) {
        this.minimumMagnitude = minimumMagnitude;
        this.maximumMagnitude = maximumMagnitude;
        this.strategy = strategy;
    }

    @Override
    public boolean satisfies(Earthquake earthquake) {
        return switch (strategy) {
            case EXCLUSIVE -> earthquake.magnitude() > minimumMagnitude &&
                earthquake.magnitude() < maximumMagnitude;
            case INCLUSIVE -> earthquake.magnitude() >= minimumMagnitude &&
                earthquake.magnitude() <= maximumMagnitude;
        };
    }
}

In some cases I would like to perform an inclusive comparison and other times I would like to perform an exclusive comparison, however in the above form, I will need to repeat the switch expression for every single implementation.

Is there an alternative way that I can generalize this pattern for all implementations so as not to have to repeat the code in the switch statement?

Tried to user another interface but ended up in a loop of repeating code again.

Ryan
  • 326
  • 2
  • 6
  • Do you mean the other filters are all just testing if a particular property of an earthquake is within some range? – Sweeper Jun 08 '23 at 12:16
  • 1
    That is **not** how you want to [compare floating point values](https://stackoverflow.com/a/4915891/2970947). – Elliott Frisch Jun 08 '23 at 12:21
  • @ElliottFrisch "Comparing for greater/smaller is not really a problem unless you're working right at the edge of the float/double precision limit." I'm not really working at the edge of the preicision limit here so I think double comparisons are fine in this scenario..? – Ryan Jun 08 '23 at 12:26
  • @Sweeper yes that's right – Ryan Jun 08 '23 at 12:27
  • @Ryan Your two filters are not comparing for greater/smaller, they only differ by comparing for exact equality. – Elliott Frisch Jun 08 '23 at 12:36

2 Answers2

3

Note that your Filter is basically identical to java.util.function.Predicate, so we can just use that. You also don't need a subclass of Filter; just use lambdas.

In your example, the inclusive/exclusive choice travels with the filter, so all you have to do is define two factories:

Predicate<Earthquake> rangeInclusive(double min, double max) { 
    return e -> e.magnitude() > min && e.magnitude() <= max;
}

Predicate<Earthquake> rangeExclusive(double min, double max) { 
    return e -> e.magnitude() > min && e.magnitude() < max;
}

If you want to use data, rather than the method name, to select inclusive/exclusive, you can do that with the switch in just one place:

Predicate<Earthquake> range(double min, double max, ComparisonStrategy strat) { 
    return switch (strat) { 
        case INCLUSIVE -> e -> e.magnitude() > min && e.magnitude() <= max;
        case EXCLUSIVE -> e -> e.magnitude() > min && e.magnitude() < max;
}
Brian Goetz
  • 90,105
  • 23
  • 150
  • 161
  • 1
    Brian this is great thank you! And an honour to hear from you, actually reading your book! – Ryan Jun 08 '23 at 12:31
  • I would argue that naming things may still be helpful in reading the code later. Perhaps, defining `interface Filter extends Predicate` is the best of both worlds. – M. Prokhorov Jun 08 '23 at 12:31
  • 2
    @M.Prokhorov As always, it depends. Sometimes domain-specific names are extremely helpful; sometimes they are just clutter. It is rare that you will know which when starting out, so don't spend a lot of time on that up front; wait until you know the shape of your solution. And in general, inventing domain-specific names for common concepts like Predicate or List is usually more in the latter category; generally best to save the domain-specific names for your actual domain concepts. – Brian Goetz Jun 08 '23 at 12:43
0

Seems to me, that the general part of your code is whether the comparison is INCLUSIVE or EXCLUSIVE, while the wrapper Filter is mostly for the property extraction.

If I'm correct, then what about doing this:

public enum ComparisonStrategy {
  INCLUSIVE() {
    @Override
    public boolean test(double value, double min, double max) {
      return value >= min && value <= max;
    }
  },
  EXCLUSIVE() {
    @Override
    public boolean test(double value, double min, double max) {
      return value > min && value < max;
    }
  };

  public abstract boolean test(double value, double rangeStart, double rangeEnd);
}

So your filter becomes:

public final class MagnitudeFilter implements Filter {

  private final double minimumMagnitude;
  private final double maximumMagnitude;
  private final ComparisonStrategy strategy;

  public MagnitudeFilter(
      double minimumMagnitude, double maximumMagnitude,
      DoubleComparisonStrategy strategy) {
    this.minimumMagnitude = minimumMagnitude;
    this.maximumMagnitude = maximumMagnitude;
    this.strategy = strategy;
  }

  @Override
  public boolean satisfies(Earthquake earthquake) {
    return strategy.test(
      earthquake.magnitude(), minimumMagnitude, maximumMagnitude
    );
  }
}

Obvious downside of this approach is if your ComparisonStrategy is used to test for more than just double values (i.e. longs and ints too) - then you either will need to extract separate DoubleComparisonStrategy, LongComparisonStrategy enum classes, or make many methods like boolean testDouble(double, double, double), boolean testLong(long, long long), etc. in the "main" ComparisonStrategy class. It may not even be possible to define a proper comparison for all possible properties you would want to compare - i.e. it's going to be a lot harder to include a Comparator<T> into this mix when going with enum approach.

UPD: combining with the approach from the other answer, this makes:

Predicate<Earthquake> magnitudeInclusive(double min, double max) {
  return e -> ComparisonStrategy.INCLUSIVE.test(e.magnitude(), min, max);
}

Predicate<Earthquake magnitudeExclusive(double min, double max) {
  return e -> ComparisonStrategy.EXCLUSIVE.test(e.magnitude(), min, max);
}

Also, this will look the same if you still want to use Filter interface, because that is also a valid lambda target:

Filter magnitudeInclusive(double min, double max) {
  return e -> ComparisonStrategy.INCLUSIVE.test(e.magnidute(), min, max);
}
M. Prokhorov
  • 3,894
  • 25
  • 39