CODEWITHSUNDEEP
javajava-8java-stream
Andrey Yaskulsky
Andrey Yaskulsky

Reputation: 2516

Java 8 streams: find items from one list that match conditions calculated based on values from another list

Have two classes and two corresponding lists:

class Click {
   long campaignId;
   Date date;
}

class Campaign {
   long campaignId;
   Date start;
   Date end;
   String type;
}

List<Click> clicks = ..;
List<Campaign> campaigns = ..;

And want to find all Clicks in clicks that:

  1. Have a corresponding Campaign in campaigns list, i.e., Campaign with the same campaignId AND

  2. This Campaign has type = "prospective" AND

  3. This Campaigns.start < click.date < Campaigns.end

So far I have the following implementation (which seems confusing and complex to me):

clicks.
        stream().
        filter(click -> campaigns.stream().anyMatch(
                campaign -> campaign.getCampaignType().equals("prospecting") &&
                        campaign.getCampaignId().equals(click.getCampaignId()) &&
                        campaign.getStart().after(click.getDate()) &&
                        campaign.getEnd().before(click.getDate()))).
        collect(toList());

I wonder if there is simpler solution for the problem.

Upvotes: 20

Views: 69557

Answers (4)

123-xyz
123-xyz

Reputation: 637

My 2 cents: Since there is no much boilerplate code in OP. So it may be not possible/necessary to reduce the lines/characters in the codes. we could rewrite it to make it a little more clearly:

Map<Long, List<Campaign>> map = campaigns.stream().filter(c -> c.type.equals("prospecting"))
                                         .collect(Collectors.groupingBy(c -> c.campaignId));

clicks.stream().filter(k -> map.containsKey(k.campaignId))
               .filter(k -> map.get(k.campaignId).stream().anyMatch(c -> c.start.before(k.date) && c.end.after(k.date)))
               .collect(Collectors.toList());

The code is not much shorter than original code. but it will improve performance from O(nm) to O(n+m), as @Marco13 mentioned in the comments. if you want shorter, try StreamEx

Map<Long, List<Campaign>> map = StreamEx.of(campaigns)
                .filter(c -> c.type.equals("prospecting")).groupingBy(c -> c.campaignId);

StreamEx.of(clicks).filter(k -> map.containsKey(k.campaignId))
        .filter(k -> map.get(k.campaignId).stream().anyMatch(c -> c.start.after(k.date) && c.end.before(k.date)))
       .toList();

Upvotes: 4

Eugene
Eugene

Reputation: 120848

Well, there is a very neat way to solve your problem IMO, original idea coming from Holger (I'll find the question and link it here).

You could define your method that does the checks (I've simplified it just a bit):

static boolean checkClick(List<Campaign> campaigns, Click click) {
    return campaigns.stream().anyMatch(camp -> camp.getCampaignId() 
               == click.getCampaignId());
}

And define a function that binds the parameters:

public static <T, U> Predicate<U> bind(BiFunction<T, U, Boolean> f, T t) {
    return u -> f.apply(t, u);
}

And the usage would be:

BiFunction<List<Campaign>, Click, Boolean> biFunction = YourClass::checkClick;
Predicate<Click> predicate = bind(biFunction, campaigns);

clicks.stream()
      .filter(predicate::test)
      .collect(Collectors.toList());

Upvotes: 7

user1803551
user1803551

Reputation: 13407

One thing that stands out is that your 2nd requirement has nothing to do with the matching, it's a condition on campaigns only. You'd have to test if this is any better for you:

clicks.stream()
    .filter(click -> campaigns.stream()
        .filter(camp -> "prospecting".equals(camp.type))
        .anyMatch(camp -> 
            camp.campaignId == click.campaignId &&
            camp.end.after(click.date) &&
            camp.start.before(click.date)
        )
    )
    .collect(Collectors.toList());

Otherwise, I have never seen a streams solution which does not involve streaming the 2nd collection inside the predicate of the 1st, so you can't do much better than what you did. In terms of readability, if it looks that confusing to you then create a method that test for the boolean condition and call it:

clicks.stream()
    .filter(click -> campaigns.stream()
        .filter(camp -> "pre".equals(camp.type))
        .anyMatch(camp -> accept(camp, click))
    )
    .collect(Collectors.toList());

static boolean accept(Campaign camp, Click click) {
    return camp.campaignId == click.campaignId &&
            camp.end.after(click.date) &&
            camp.start.before(click.date);
}

Finally, 2 unrelated suggestions:

  1. Don't use the old Date class, instead use the new java.time API's LocalDate.
  2. If Campaign's type can only have some predefined values (like "submitted", "prospecting", "accepted"...) then an enum would be a better fit than a general String.

Upvotes: 6

albert_nil
albert_nil

Reputation: 1658

public List<Click> findMatchingClicks(List<Campaign> cmps, List<Click> clicks) {
    List<Campaign> cmpsProspective = cmps.stream().filter(cmp -> "prospective".equals(cmp.type)).collect(Collectors.toList());
    return clicks.stream().filter(c -> matchesAnyCmp(c, cmpsProspective).collect(Collectors.toList());
}

public boolean matchesAnyCmp(Click click, List<Campaign> cmps) {
     return cmps.stream().anyMatch(click -> cmp.start.before(click.date) && cmp.end.after(click.date));
}

Replace fields for getters, just wrote it quick.

Upvotes: 1

Related Questions