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?

Answer 1

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().

Answer 2

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.