Contradictory conditions

[johnjohndoe]

Context

The public interface of the library as of version 0.16 allows to either use DefaultRateSnack() or to extend from the class. DefaultRateSnack() ships with predefined conditions:

public DefaultRateSnack() {
    withConditions(new NeverAgainWhenClickedOnce(), 
                   new AfterNumberOfOpportunities(5), 
                   new IsConnectedViaWiFiOrUnknown());
}

All conditions are added in BaseSnack class:

public BaseSnack withConditions(SnackCondition... conditions) {
    Collections.addAll(conditionList, conditions);
    return this;
}

The conditionList is evaluated in opportunity() in the BaseSnack class:

@Override
public boolean opportunity(final SnackContext snackContext) {
    this.snackContext = snackContext;

    for (final SnackCondition snackCondition : conditionList) {
        if (!snackCondition.isAppropriate(snackContext, this)) {
            return false;
        }
    }

   // ... show snack

Simplified it can be said that all conditions must be met in order to show the snack.

A custom rate snack

As stated before users of the library can inherit from DefaultRateSnack. One use case is to change the conditions under which a rate snack is shown. Here is an example:

class CustomRateSnack extends DefaultRateSnack {

    public CustomRateSnack() {
        withConditions(
                new AfterNumberOfOpportunities(8)
        );
    }

}

The new condition is added to conditionList as before and is evaluated in opportunity() in the BaseSnack class.

The issue with conditions

Both the conditions of the base DefaultRateSnack class and its subclass CustomRateSnack are added to the conditionList. This results in the following condition list:

NeverAgainWhenClickedOnce(), AfterNumberOfOpportunities(5), IsConnectedViaWiFiOrUnknown(), AfterNumberOfOpportunities(8)

As one can see the list now contains two AfterNumberOfOpportunities conditions which contradict with one another. When being evaluated the "stronger" condition AfterNumberOfOpportunities(8) wins in the particular example.

Proposals

The given example shows that adding the same conditions multiple times might lead to misunderstanding or even misconfiguration. Here are different proposals to mitigate the issue:

1. Disallow to inherit from DefaultRateSnack by making the class final. Users can extend from the RateSnack class.
2. Automatically replace any existing condition of the same type once a new one instance is added.

Related

• Issue #8: Multiple Snacks with same condition