azuyalabs / yasumi

The easy PHP Library for calculating holidays
https://www.yasumi.dev
Other
1.04k stars 152 forks source link

Between Filter should have a ->count that is correct #77

Closed creuzerm closed 6 years ago

creuzerm commented 6 years ago

I think the between filter should output the correct count of dates as found by the filter. (Likely desired behavior for all filters, I haven't used any of the others yet)


$holidays = Yasumi\Yasumi::create('USA', 2017);
$holidays_Today = $holidays->between(new DateTime("11/01/2017"), new DateTime("12/31/2017"));
echo $holidays_Today->count(); 

11

I would expect this value to be 3 based on the holiday dates calculated.

stelgenhof commented 6 years ago

Do you mean the between filter currently doesn't return the correct number of holidays?

creuzerm commented 6 years ago

Correct. The between filter doesn't return the correct number of holidays. In the example I gave above, it is returning 11 when I expect 3.

stelgenhof commented 6 years ago

Oh, that's not good. Let me check that.

stelgenhof commented 6 years ago

The between Filter is based on the FilterIterator class and returns an Iterator Object. The ->count() function doesn't get propagated to the an Iterator class (See also this bug report: https://bugs.php.net/bug.php?id=70113).

If you use the iterator_count function it will return the actual (filtered) number of holidays, like this:

echo iterator_count($holidays_Today): // 3

stelgenhof commented 6 years ago

Following up on my previous comment: I've made a quick test implementing the Countable Interface and looks like it's working if you're using the count() function. I just need to create some unit tests and apply to the other filters as well.

creuzerm commented 6 years ago

Thank you. Would adding an example of this to the filter cookbook page make sense? It would be helpful for people like me that are only fair coders and dont know what to expect from the iterator class.

stelgenhof commented 6 years ago

I have implemented the Countable interface allowing you to use the ->count() method. For the moment I have only done it for the BetweenFilter (in the master branch). The other filters will be updated soon and also the documentation :)

Please have a look and let me know how it goes :)

stelgenhof commented 6 years ago

All filters have now the Countable interface implemented. Available in the master branch and soon also in the upcoming v1.7.0 release.