can collections.filter be wrong?

Question

I try to filter a collection according to a predicate:

                private void filterExpiredOffers() {
                    mOffersList = Lists.newArrayList(Collections2.filter(
                            mOffersList, new Predicate<Offer>() {
                                @Override
                                public boolean apply(Offer offer) {
                                    return mUnlockExpirationCalculator
                                            .isUnlockValid(offer);
                                }
                            }));
                }

and:

public boolean isUnlockValid(Offer offer) {
    return ((offer.unlockExpirationDate == null) || (System
            .currentTimeMillis() < offer.unlockExpirationDate.getTime()));
}

I see an offer has gotten a "false" as a result

but yet, I see it in the arrayList later on.

Am I doing the filter wrong?

enter image description here

(Basically, the answer is: no, `Collection2.filter` cannot be wrong. The implementation is quite simple, and when iterating over the `Collection` it will only return elements for which `predicate.apply` returns `true`.) — ColinD, Sep 07 '14 at 16:21

maaartinus · Answer 1 · 2014-09-07T10:01:54.423

You really shouldn't do things like this:

public boolean isUnlockValid(Offer offer) {
    return ((offer.unlockExpirationDate == null) || (System
            .currentTimeMillis() < offer.unlockExpirationDate.getTime()));
}

Create a class instance instead, which captures System.currentTimeMillis() and uses it. This way your filter will stay stable over time.

Consider something like this

class UnlockValidPredicate implements Predicate<Offer> {
    public UnlockValidPredicate() {
        this(System.currentTimeMillis());
    }

    public UnlockValidPredicate(long millis) {
        this.millis = millis;
    }

    @Overrride public boolean apply(Offer offer) {
        return offer.unlockExpirationDate == null
                || millis < offer.unlockExpirationDate.getTime();
    }

    private final long millis;
}

Also consider getting rid of nulls. Setting unlockExpirationDate to new Date(Long.MAX_VALUE) is good enough for "never expire", isn't it?

That's not it. The current time was Sep 7th. The unlockExpirationDate was Aug 30th. It's not a matter of days between the filtering and the debugging i did. what else can it be?

Get rid of Date, it's stupid mutable class. Most probably, you changed it somehow. Things like

 MyClass(Date date) {
     this.date = date;
 }

and

 Date getDate() {
     return date;
 }

are a recipe for disaster. The best solution is to use an immutable class (available in Java 8 or JodaTime). The second best is to use long millis. The last is to clone the Date everywhere.

That's not it. The current time was Sep 7th. The unlockExpirationDate was Aug 30th. It's not a matter of days between the filtering and the debugging i did. what else can it be? — Elad Benda, Sep 07 '14 at 07:00
anyhow why is my predicate returns true? (even though i couldn't catch it during debugging) — Elad Benda, Sep 07 '14 at 18:42
@EladBenda `millis` is immutable. Unless you somehow do `millis = ...`. it won't change. Doing `Date now = new Date(); MyClass = new MyClass(now); now.set(....);` or similar blows and that's my guess of what's happening. — maaartinus, Sep 07 '14 at 22:44

score 2 · Answer 2 · answered Sep 06 '14 at 23:14

2

What seems most likely is that the predicate was true when you did the filtering, and then the predicate became false later -- which seems perfectly possible when you're using System.currentTimeMillis().

answered Sep 06 '14 at 23:14

Louis Wasserman

191,574
25
345
413

That's not it. The current time was Sep 7th. The unlockExpirationDate was Aug 30th. It's not a matter of days between the filtering and the debugging i did. what else can it be? – Elad Benda Sep 07 '14 at 06:48

can collections.filter be wrong?

2 Answers2