Skip to content

Conversation

@nyamsprod
Copy link
Contributor

@derickr here's the PR to add DatePeriod::getRecurrences I hope I've follow your guidance correctly If something is missing please let me know.


dpobj = Z_PHPPERIOD_P(ZEND_THIS);

if (!dpobj->recurrences) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@derickr seems dpobj->recurrences by default equals 1 instead of NULL maybe we should substract 1 to the value and return if the substracted value equals 0 🤔


dpobj = Z_PHPPERIOD_P(ZEND_THIS);

if (0 == dpobj->recurrences - 1) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's incorrect. By default, the start date is also included as a recurrence.

If you set the DatePeriod::EXCLUDE_START_DATE option, then this does not happen.

So instead of using - 1, you should be using - dpobj->include_start_date.

It's probably wise to add a test case for this too.

string(12) "+0-0-1-0-0-0"
NULL
int(4)
NULL
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make sure the last line has a new line too.

@derickr derickr changed the base branch from master to PHP-7.2 March 17, 2019 18:33
@derickr derickr changed the base branch from PHP-7.2 to master March 17, 2019 18:34
@derickr
Copy link
Member

derickr commented Mar 17, 2019

Committed and merged after rebasing this to PHP-7.2

@derickr derickr closed this Mar 17, 2019
@carusogabriel
Copy link
Contributor

@derickr Why putting a new method into PHP-7.2?


$periodWithRecurrencesWithoutStart = new DatePeriod($start, $interval, $recurrences, DatePeriod::EXCLUDE_START_DATE);

var_dump($periodWithRecurrences->getRecurrences());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo? I assume this should be $periodWithRecurrencesWithoutStart instead.

@nyamsprod nyamsprod deleted the bugfix/adding-DatePeriod-getRecurrences-method branch April 17, 2019 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants