Guard `count` argument in take/[2,3]

pbogut / recurring_events

Elixir library for dealing with recurring events

MIT License

26 stars 8 forks source link

Guard `count` argument in take/[2,3] #18

Closed voughtdq closed 5 years ago

voughtdq commented 5 years ago

Giving a negative value to take/[2,3] would use VM resources until the VM crashed. A guard was added to ensure count is 0 or greater and a test was added to verify take[2,3] raises when given a negative value.

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 77

3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 100.0%

Totals
Change from base Build 70:	0.0%
Covered Lines:	281
Relevant Lines:	281

💛 - Coveralls

pbogut commented 5 years ago

@voughtdq Thank you for submitting pull request.

This fix will work with take, but I think it also should be checked when unfold. If you tak a look at that file there is a validate/2 function that suppose to raise errors when rules are incorrect.

Would you mind change this PR to perform check within validate/2 function, and raise error there instead of using guard on take/3 please?

voughtdq commented 5 years ago

If a guard isn't used on take/3, it will always crash the VM. Here's another example:

~D[2015-09-13] |> RecurringEvents.unfold(%{freq: :monthly}) |> Enum.take(-1)

In other words, this will unfold forever trying to find the last element in the list.

pbogut commented 5 years ago

@voughtdq sorry, this was me being stupid, thought count is part of the rules but it is not in that case.