Provide support for DateTimes in DateRange

[rquadling]

Currently DateRange only deals with dates. diff() amends date by a day and __toString() outputs only the date.

DateTimeRange overrides these 2 methods and processes times.

All unit tests cloned from DateRangeTest, times added and all pass.

[rquadling]

I can't see what the conflicts are. Unless it is the lock file. Which should be removed.

[gws]

Yeah, I removed the lockfile in 2cf7dfa before I saw your PR.

[rquadling]

Any chance of a merge of this PR?

[gws]

@rquadling Yeah, I think so. I'm low on free time, but should have some time later this week. I will probably be merging https://github.com/gws/php-valueobjects/tree/use-datetimeinterface soon once I polyfill DateTimeImmutable::createFromMutable, and that branch will conflict with this PR.

[rquadling]

DateTimeImmutable cannot polyfill to PHP 5.4 due to DateTime class not being able to implement DateTimeInterface. I don't want to swap out the use of \DateTime for my own class as that changes WAY too much code.

Bit stuck with this. I have to continue using 5.4 (going to 7 this year sometime).

If you could merge my PR and tag it before use-datetimeinterface, then I can fix my dependency to that tag. As soon as we are 7, then we can move our dependency up to the latest stable.

[gws]

I see. It's no problem to hold off on merging that branch and bumping the requirement to 5.5, then.

[gws]

Using \Z here is a lie if the DateTimes aren't in that timezone. Something like $this->getStart()->format('c') would be better.

[rquadling]

I think the output has to be timezone neutral, so maybe using DateTime::setTimezone(new DateTimeZone('UTC')) would be more appropriate.

[gws]

That would be valid in the sense that it would at least represent the correct instant in time, but it doesn't respect the time zone(s) that the user created the range with. Non-UTC timezones are perfectly valid, and in some situations (e.g. debugging a calendaring/scheduling app), they may be desired.

[rquadling]

https://en.wikipedia.org/wiki/ISO_8601#Time_intervals and mentions the 4 ways and none of them are tz specific. As the format being represented is an interval, there is no relevance to the timezone.

And https://www.cs.tut.fi/~jkorpela/iso8601.html#sol, and specifically, "Period of time format", also use Zulu format.

But having said that, http://www.uai.cl/images/sitio/biblioteca/citas/ISO_8601_2004en.pdf (Section 4.4 page 20) makes no mention of the Z and explicitly refers to "local time".

So, no \Z part if not UTC timezone.

[rquadling]

I think. It's late.

[gws]

If I create a DateTimeRange as

$dr = new DateTimeRange(
  new DateTime('2016-01-01T00:00:00-0700'),
  new DateTime('2016-01-01T04:00:00-0400')
);

That's a 1 hour interval and I'd expect __toString() to print 2016-01-01T00:00:00-0700/2016-01-01T04:00:00-0400, which is perfectly valid (example 1 here: https://en.wikipedia.org/wiki/ISO_8601#Time_intervals).

[rquadling]

OK. It is annoying that none of the documentation for time intervals includes an example with timezone offset, just zulu time.

I'll update the code to use 'c' type. At least that does fix the issue for non UTC users.

[gws]

Can we type hint on DateTimeRange here as well as updating any relevant docstrings?

[rquadling]

The type hint has to remain as a DateRange, of which a DateTimeRange is a descendent. I've added a unit test for DateRange->diff(DateTimeRange) and DateTimeRange->diff(DateRange).

[gws]

@rquadling I think I'm done with review - if we can resolve my most recent comment I'll merge this.

[rquadling]

Finally!!!

[gws]

@rquadling OK, merged. Thank you!

[rquadling]

Thank you.