Handling a a logical inconsistency RRULE

[chungonn]

Hi,

First, I would like to thank you for nice new API for creating recurrenceSet, it is very intuitive and flexible. Though there are breaking change from 0.15.0 to 0.17.1, the improvements in the library are worthy of the upgrade.

This issue existed since 0.15.0. Given an illogical rrule caused by BYPOS=2
"FREQ=WEEKLY;INTERVAL=2;BYDAY=SA;BYHOUR=19;BYMINUTE=0;BYSECOND=0;BYSETPOS=2"

The RecurrenceSet#hasNext or RecurrenceSet#next will throw the following exception below. In this situation, should the InstanceIterator a false or null instead?

java.lang.IllegalStateException: too many empty recurrence sets

at org.dmfs.rfc5545.recur.BySetPosFilter.nextSet(BySetPosFilter.java:88)
at org.dmfs.rfc5545.recur.BySetPosFilter.next(BySetPosFilter.java:69)
at org.dmfs.rfc5545.recur.RecurrenceRuleIterator.fetchNextInstance(RecurrenceRuleIterator.java:95)
at org.dmfs.rfc5545.recur.RecurrenceRuleIterator.<init>(RecurrenceRuleIterator.java:89)
at org.dmfs.rfc5545.recur.RecurrenceRule.iterator(RecurrenceRule.java:2218)
at org.dmfs.rfc5545.recurrenceset.OfRule.iterator(OfRule.java:42)
at org.dmfs.jems2.iterator.Mapped.next(Mapped.java:64)

[dmfs]

I'm glad to hear that you enjoy the new API.

I've spent quite some time thinking about this issue. Unfortunately it's not trivial to separate rules that never emit an occurrence from those that do so every couple of thousand occurrences (see #48 for an example of that).
In case of BYSETPOS we'd have to decide whether the rule ever has that many instance in an interval. That alone can be a challenging task. Just think of something like

FREQ=WEEKLY;BYMONTH=2;BYMONTHDAY=15,29;BYDAY=SA;BYSETPOS=2

This rule doesn't make much sense and certainly can be simplified. However, the rule is valid. Yet only every 28 years / ~1450 weeks there is a February in which the 29th falls on a Saturday. You can easily modify it to never emit anything.

I see three possible solutions right now:

1. keep the code as it is and live with those edge cases not being treated properly
2. throw a custom (Runtime-) Exception instead of a generic one, so the caller can at least (reliably) distinguish that case from another IlleagalStateException that might occur
3. Implement some code to recognize (at least some of) those edge cases and stop iterating without an error when no results are to be expected.

I wonder if the implementation effort and the runtime overhead are worth implementing number 3.

[dmfs]

I've added #138 to cover option 2.

[chungonn]

Hi Marten,

Thanks for sharing the challenges and your thoughts about the issue.

The first thing I thought when I had the problem was, wouldn't it be nice if there is a RecurrenceRule#isValid. And there is such a method but it is in private. It however checks for the well-formed-ness of the RRule which did not handle the logic error in my case. If there is such method to check both well-formed and logical error than the onus is on the user of the library.

I do agree with you that there are always so many things to do and we must put our energy and time on what provide the best return.

If I may suggest a simpler solution to my problem. If there is a logical error in the RRule then RecurenceSet#iterator#hasNext will return false as there is nothing to return correspondingly for RecurrenceSe#iterator#next will return null. Perhaps there is an addition state to hold the error if any for diagnostic purposes.

With this, even with an illogical RRule, the user will not get any errors but instead get a null for next and a false for hasNext especially in runtime, maybe frowned upon by some of the library users though,

[dmfs]

Hmm, the suggested behavior would violate the Iterator Contract. I'd rather not do that.

[chungonn]

Hi Marten, I agreed adhering to the specs is important. From the JavaDoc, it seems to imply that hasNext should not throw an exception. In my case, the hasNext method does throw the same exception as the method next(). Would you consider that hasNext should return a false instead of throwing an exception for the condition I raised previously?

…

On Wed, May 22, 2024 at 12:22 AM Marten Gajda ***@***.***> wrote: Hmm, the suggested behavior would violate the Iterator Contract <https://docs.oracle.com/javase/8/docs/api/java/util/Iterator.html#next-->. I'd rather not do that. — Reply to this email directly, view it on GitHub <#137 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACNXKQ2A6ECJNETAYRGHTDZDNYFXAVCNFSM6AAAAABH5FZRFKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRSHE4TINRYG4> . You are receiving this because you authored the thread.Message ID: ***@***.***>