Match >= for compareTo if lifecycle events are Comparable #196
Conversation
Resolves #192, but I'm not totally sure this is the best approach as it assumes all enum-defined lifecycles go in order. As a convention, I can't think of any examples that don't abide by this. But if one ever did, it would no longer work. Possible solutions if this is an issue in the future: Add a plugin hook to define custom enum boundary check, just say that if natural ordering fallthrough is not the right complexity then they should probably make a full class instead, or just make a simpler ScopeProvider.
|
To be clear, I think we should move forward with this approach. I just think we're probably facing a trade-off whatever direction we take and would rather lean in the direction that does what users expect most of the time |
|
Makes sense to me - one question though, can this have adverse effects for people upgrading? |
|
Going to punt on this for the 0.8.0 release while we figure out the behavior of this a bit more. Can do a bugfix release with what we land on easily though |
| // Match on enum ordinal in case we skip an event | ||
| final int targetOrdinal = ((Enum) endEvent).ordinal(); | ||
| //noinspection unchecked | ||
| equalityFunction = (Function<E, Boolean>) new Function<Enum, Boolean>() { |
There was a problem hiding this comment.
All java enums implement Comparable. You can use compareTo to get this behavior by default and allow consumers to provide their own if using custom enum events.
There was a problem hiding this comment.
So should we check if it implements comparable instead of being an instance of enum?
ZacSweers
changed the title
Resolves #192, this checks if the lifecycle even type is comparable and uses it in comparison checking. This works for enums as a convention, I can't think of any examples that don't abide by this, but also allows for overriding the behavior if necessary. The idea here is that if we target end event "B" from start event "A", any events after "B" should also be counted as stop points.
CC @jinatonic