Add DateInterval::add and DateInterval::sub methods

[bukka]

Adding and substraction of two intervals

[laruence]

could you also add some test scripts?

[kaplanlior]

@bukka , thanks for the fast response (:

[weltling]

Tested and it's good so far on linux/windows. Though a couple of questions

• if I add two intervals of 9999 years, i see a year with 5 places. Isn't it out of range?
• I currently don't see a way to substract intervals. Tried to set invert = 1 still results in an addition. Adding a sub method might be an option too.

I think that two things should be fixed as they'll flood back anyway as bugs. Also please add more tests for such edge cases.

[bukka]

I have just added a sub method for subtracting.

I will have a look into the invert and add more tests for edge cases (it would be great if you could suggest some).

Not sure if I understand why year with 5 places is out of range? It seems still better than warning...?

Thanks for the feedback ;)

[weltling]

With the year - that was a question ;) Rereading the docs it seems to be fine for Durations, see http://en.wikipedia.org/wiki/Iso8601#Years

The invert option is is use for instance when substracting an interval for a date. DateTime::add() is about how it works.

For the other edge cases - look through the docs. Normally timelib should do its work, but things happen, you know.

[bukka]

Just added support for invert.

It could be actually useful to add support for operators overloading (as it is already in gmp) and then do something like:
$inverval = $interval1 + $interval2
It looks much nicer.

What do you think?

[weltling]

Testing a bit looks like the current implementation doesn't work correctly. Please consider the following snippets

$i0 = new Dateinterval('P1Y4D');
$i1 = new DateInterval('P1Y3D');
$i0->invert = 1;
$i2 = $i0->add($i1); /* -1 day */
var_dump($i2->format('%y-%m-%d %h-%i-%s'));
var_dump($i2->invert);

$i2 = $i0->sub($i1); /* -(2 years and 7 days) */
var_dump($i2->format('%y-%m-%d %h-%i-%s'));
var_dump($i2->invert);

$i0->invert = 0;
$i2 = $i0->sub($i1);
var_dump($i2->format('%y-%m-%d %h-%i-%s'));
var_dump($i2->invert);

Current output:
string(11) "2-0-7 0-0-0"
int(0)
string(12) "2-0--7 0-0-0"
int(1)
string(11) "0-0-1 0-0-0"
int(0)

Expected output:
string(11) "0-0-1 0-0-0"
int(1)
string(11) "2-0-7 0-0-0"
int(1)
string(11) "0-0-1 0-0-0"
int(0)

$i0 = new Dateinterval('P1Y4DT3H');
$i1 = new DateInterval('P1Y4DT3H2M');
$i2 = $i0->sub($i1); /* -2 minutes */
var_dump($i2->format('%y-%m-%d %h-%i-%s'));
var_dump($i2->invert);

$i2 = $i1->sub($i0);
var_dump($i2->format('%y-%m-%d %h-%i-%s'));
var_dump($i2->invert);

Current output
string(13) "0-0-1 23-58-0"
int(1)
string(11) "0-0-0 0-2-0"
int(0)

Expected output
string(11) "0-0-0 0-2-0"
int(1)
string(11) "0-0-0 0-2-0"
int(0)

These tricky cases seems to be not covered, please correct me when i'm wrong. It seems not to respect the overflows in subordinate units, like if there where 61 second, it should go up to 1 minute and 1 second. Also i think the logic in the last commit is too complicated, please try to avoid that nested if statements, making it flat will result more feasible code. The thing is much more simple )

About the operator overloading - AFAIR there is none of that in any of the datetime classes, so it were better to stay in what this PRs title says. The operator overloading should be done in one move where it's applicable for datetime, IMHO.

Thanks for your effort.

[bukka]

Thanks for testing. I fixed all mentioned problems and added more tests. Also the code looks a bit better now.

[weltling]

This one is merge ready i think, good job!

[derickr]

Please, no macros. They are a pain to debug and it's really annoying if their names are lower case too!

[derickr]

s a concept, I think this is not really a good idea. You can add normal
intervals such as y/m/d/h/i/s, but how would you add things to "next
weekday" or "friday 3 weeks" etc to each other? If there is a add/sub,
then that really needs to work as well and as it's virtually impossible to do that right... I'd be against adding this.

[php-pulls]

Comment on behalf of krakjoe at php.net:

This PR is not in very good shape today, the objections raised by datelib maintainer do not seem to be addressed, indeed seem to point out problems that are intractable, I'm closing this PR.

Since the objection from derick is so strong, and since there seems to be theoretical problems with adding this kind of functionality, I'm not sure if it's worth opening a clean PR.