pbogut / recurring_events

Elixir library for dealing with recurring events
MIT License
26 stars 8 forks source link

Exclude DateRange #11

Closed pinx closed 3 years ago

pinx commented 6 years ago

Ref #10

The added test is failing, because the expected behavior is not implemented. If I ask for 5 recurrences, for a daily frequency, and 3 of these dates are excluded, I expect to get 2 dates. I shouldn't have to know beforehand about the effect of exclusions.

How would we solve this?

The RecurringEvents.take function could be changed to first do the Enum.take and then apply the drop_exclude.

coveralls commented 6 years ago

Pull Request Test Coverage Report for Build 63


Totals Coverage Status
Change from base Build 62: 0.0%
Covered Lines: 280
Relevant Lines: 280

💛 - Coveralls
coveralls commented 6 years ago

Pull Request Test Coverage Report for Build 64


Totals Coverage Status
Change from base Build 62: 0.0%
Covered Lines: 280
Relevant Lines: 280

💛 - Coveralls
pbogut commented 6 years ago

I would argue that if definition is endless stream and you want 5 dates, then you want 5 datets. I would always expect take with endless stream and param 5 to return 5 elements. Why do you think it should return only 2 elements? Do you have any examples of simmilar libs working this way?

I've changed it in https://github.com/pbogut/recurring_events/commit/8d31c462fffae84493c61acf2495d20645ab38ee because I wanted it to work this way. I cant find clear information in RFC2445 sepecification about that when should it be excluded. If its common practice to implement it the way you say then I may bo convinced to do that, but would need some more information.

pinx commented 6 years ago

I think you are right. You can use Enum.take_while to get dates until an end date. By combining this with Date.add(), you can get all dates within a date range, e.g. for a number of days.

I updated the test, and all tests pass now.