3

Given the following code:

stream.filter(o1 -> Objects.equals(o1.getSome().getSomeOther(),
                                   o2.getSome().getSomeOther())

How could that possibly be simplified?

Is there some equals-utility that lets you first extract a key just like there is Comparator.comparing which accepts a key extractor function?

Note that the code itself (getSome().getSomeOther()) is actually generated from a schema.

Roland
  • 22,259
  • 4
  • 57
  • 84
  • 1
    Almost a duplicate of [Is there a convenience method to create a Predicate that tests if a field equals a given value?](http://stackoverflow.com/questions/33300011/is-there-a-convenience-method-to-create-a-predicate-that-tests-if-a-field-equals), except here you want to apply the `Function` on `o2` as well. – Didier L Apr 27 '17 at 14:59

2 Answers2

2

EDIT: (after discussing with a collegue and after revisiting: Is there a convenience method to create a Predicate that tests if a field equals a given value?)

We now have come to the following reusable functional interface:

@FunctionalInterface
public interface Property<T, P> {

  P extract(T object);

  default Predicate<T> like(T example) {
     Predicate<P> equality = Predicate.isEqual(extract(example));
     return (value) -> equality.test(extract(value));
  }
}

and the following static convenience method:

static <T, P> Property<T, P> property(Property<T, P> property) {
  return property;
}

The filtering now looks like:

stream.filter(property(t -> t.getSome().getSomeOther()).like(o2))

What I like on this solution in respect to the solution before: it clearly separates the extraction of the property and the creation of the Predicate itself and it states more clearly what is going on.

Previous solution:

<T, U> Predicate<T> isEqual(T other, Function<T, U> keyExtractFunction) {
  U otherKey = keyExtractFunction.apply(other);
  return t -> Objects.equals(keyExtractFunction.apply(t), otherKey);
}

which results in the following usage:

stream.filter(isEqual(o2, t -> t.getSome().getSomeOther())

but I am more then happy if anyone has a better solution.

Community
  • 1
  • 1
Roland
  • 22,259
  • 4
  • 57
  • 84
  • 2
    You should apply the function on `other` outside the lambda, so that it is called only once on `other`. For the moment you call it for every `t`. – Didier L Apr 27 '17 at 15:01
  • 1
    I think the code in your question is much clearer than the code enabled by your answer. `isEqual` is just way too vague. – ruakh Apr 27 '17 at 15:28
  • @ruakh I think this would be more readable with method references, e.g. `isEqual(o2, MyObject::getSome)`. But this does not work with chained calls. – Didier L Apr 27 '17 at 16:15
  • @ruakh right, somehow... what I don't like is the repeated call and as @DidierL stated it gets nicer if method references are used. The benefit of that function is that whenever you compare something within a filter you have the possibility of just saying `isEqual(otherObject, ObjectType::getValueThatShouldBeEqual)` and can easily switch what you would like to compare if you need to (editing only 1 part, instead of 2). – Roland Apr 28 '17 at 07:15
  • 1
    You should follow the link, Didier L has posted in [this comment](http://stackoverflow.com/questions/43661288/java-8-streams-simplifying-o1-objects-equalso1-getsome-getsomeother-o2#comment74369715_43661288). The interesting point of the answer there is, that you don’t need `Objects.equals`, when you know `otherKey` beforehand. `otherKey==null? t -> keyExtractFunction.apply(t)==null: t->otherKey.equals( keyExtractFunction.apply(t))` makes the per-element evaluation more efficient. In fact, now that you factored out the function evaluation for the constant, the remaining task is identical. – Holger Apr 28 '17 at 09:33
  • ah... nice... I just oversaw that on first read... that makes sense indeed! – Roland Apr 28 '17 at 11:08
1

I think that your question's approach is more readable than your answer's one. And I also think that using inline lambdas is fine, as long as the lambda is simple and short.

However, for maintainance, readability, debugging and testability reasons, I always move the logic I'd use in a lambda (either a predicate or function) to one or more methods. In your case, I would do:

class YourObject {

    private Some some;

    public boolean matchesSomeOther(YourObject o2) {
        return this.getSome().matchesSomeOther(o2.getSome());
    }
}

class Some {

    private SomeOther someOther;

    public boolean matchesSomeOther(Some some2) {
        return Objects.isEqual(this.getSomeOther(), some2.getSomeOther());
    }
}

With these methods in place, your predicate now becomes trivial:

YourClass o2 = ...;

stream.filter(o2::matchesSomeOther)
fps
  • 33,623
  • 8
  • 55
  • 110
  • I do create new methods if it makes sense.. regarding filters I clearly prefer some way that is still understandable, but also reusable. I added another approach to my answer that, at least for me, is still understandable and reusable for use cases where only properties need to be matched. – Roland Apr 28 '17 at 09:24
  • @Roland It is a matter of taste. I prefer to separate the flow (given by the stream pipeline) from the extraction of the logical conditions that drive the pipeline. Same with mappings and calculations. Your new approach (now that you have edited your answer) looks good too. – fps Apr 28 '17 at 12:41
  • @FedericoPeraltaSchaffner yes, you are right. it's a code smell that violate [LoD](https://en.wikipedia.org/wiki/Law_of_Demeter) principle, so you have a my up-vote. and maybe could design the OP's code in OO way to avoiding train-wrecks code that introduced in GOOs. – holi-java Apr 30 '17 at 06:19
  • 1
    as a side note: the code for `getSome().getSomeOther()` is actually generated from a schema. That's also the reason I searched for a way to extract those properties rather then correcting the code to become more OO. I could have used wrappers for that too, but that is just way more overhead for what I am actually using it now ;-) – Roland May 01 '17 at 08:29
  • 1
    @Roland Now that makes perfect sense :) Your approach is functional style, and OO and functional don't always match in their principles and design patterns. – fps May 01 '17 at 11:13