Inclusive options on `GetNextOccurrences`

[mrxrsd]

Hi,

Is there any tricky reason to baseTime/endTime not be inclusive? or it was just a design decision ?

I was thinking to create a PR with a new api proposal :

IEnumerable<DateTime> GetNextOccurrences(DateTime baseTime, DateTime endTime, bool baseTimeInclusive = false, bool endTimeInclusive = false);

Do you think it can be a nice feature?

[atifaziz]

It was a design decision that avoids bugs in the simplest use cases without additional thought and effort. It's easier to start with GetNextOccurrence where the problem is more obvious. The goal with GetNextOccurrence was that you can feed its previous result into a subsequent invocation to, for example, loop through occurrences. It should be possible to do that in simple loop constructs like for, while, etc. If the base time was inclusive, you would get an infinite loop since GetNextOccurrence would keep returning the same value. If you look at how GetNextOccurrences is implemented, it's a very simple for-loop that builds on GetNextOccurrence:

https://github.com/atifaziz/NCrontab/blob/9b68c8d1484ccd56a8f0bc1ce12e7270736f3493/NCrontab/CrontabSchedule.cs#L178-L186

It actually calls a private version named TryGetNextOccurrence, but it's practically identical to GetNextOccurrence since the latter delegates to the former:

https://github.com/atifaziz/NCrontab/blob/9b68c8d1484ccd56a8f0bc1ce12e7270736f3493/NCrontab/CrontabSchedule.cs#L210-L211

I don't think an additional API will add much value. Creating an inclusive start is as simple as subtracting a tick. Likewise, you can create an inclusive end by adding a tick. For example:

var schedule = CrontabSchedule.Parse("0 */6 * * *");
var start = new DateTime(2022, 10, 6).AddTicks(-1); // inclusive start
var end = start.AddDays(1).AddTicks(1); // inclusive end
var occurrences = schedule.GetNextOccurrences(start, end);
Console.WriteLine(string.Join(Environment.NewLine, occurrences));

// Prints (assuming English, United Kingdom, settings):
// 06/10/2022 00:00:00
// 06/10/2022 06:00:00
// 06/10/2022 12:00:00
// 06/10/2022 18:00:00

The end-time is exclusive by design too, to avoid surprising bugs. Suppose you want to print in batches of occurrences per day:

var schedule = CrontabSchedule.Parse("0 */6 * * *");
var date = new DateTime(2022, 10, 6);
for (var i = 0; i < 4; i++)
{
    var start = date.AddTicks(-1); // inclusive
    var end = date.AddDays(1); // exclusive
    var occurrences = schedule.GetNextOccurrences(start, end);
    Console.WriteLine(string.Join(", ", occurrences));
    date = end;
}

// Prints (assuming English, United Kingdom, settings):
// 06/10/2022 00:00:00, 06/10/2022 06:00:00, 06/10/2022 12:00:00, 06/10/2022 18:00:00
// 07/10/2022 00:00:00, 07/10/2022 06:00:00, 07/10/2022 12:00:00, 07/10/2022 18:00:00
// 08/10/2022 00:00:00, 08/10/2022 06:00:00, 08/10/2022 12:00:00, 08/10/2022 18:00:00
// 09/10/2022 00:00:00, 09/10/2022 06:00:00, 09/10/2022 12:00:00, 09/10/2022 18:00:00

If the end was not exclusive, the first line would end up including 07/10/2022 00:00:00. Because the end is exclusive, at the end of each iteration in the above code, date is simply set to end (which was excluded) and on next iteration that gets included by subtracting a tick.

Hope this helps explain the design choices.

Now if someone wants to still have an API the way you're proposing then it could be added as a simple extension method in their project:

static class Extensions
{
    public static IEnumerable<DateTime>
        GetNextOccurrences(this CrontabSchedule schedule,
                           DateTime baseTime, bool includeBaseTime,
                           DateTime endTime, bool includeEndTime) =>
        schedule.GetNextOccurrences(includeBaseTime ? baseTime.AddTicks(-1) : baseTime,
                                    includeEndTime ? endTime.AddTicks(1) : endTime);
}

The reason I'd hesitate to add it to NCrontab is that in the 14 years I've been maintaining the project, it's not something that's been requested by many users so that says one of two things:

• It's not commonly needed.
• People are getting by fine with their own local helpers (like the extension I showed above).

[mrxrsd]

makes senses! In the end I've come up with the same solution. Thanks for ur time.