search

brick / date-time

Date and time library for PHP
MIT License
335 stars 30 forks source link

Add OffsetDateTime #12

Closed joshdifabio closed 5 years ago

joshdifabio commented 5 years ago

We currently have a need for an OffsetDateTime in java.time parlance. Is there any reason not to add it?

BenMorel commented 5 years ago

IIRC, I did not add it at the time because you can achieve the same thing using ZonedDateTime with a TimeZoneOffset.

Did I miss something?

joshdifabio commented 5 years ago

Aha. In Java these are distinct things -- a ZonedDateTime always has a 'ZoneId` (e.g. London) and an OffsetDateTime never does.

I assumed you'd taken the same approach but I see now that here the two are combined into a single class. I think it's a shame to lose the specificity that java.time provides but you're right, we can use ZonedDateTime in our case. Thanks.

BenMorel commented 5 years ago

I think it's a shame to lose the specificity that java.time provides

Can you please elaborate?

joshdifabio commented 5 years ago

I think it's a shame to lose the specificity that java.time provides

Can you please elaborate?

In Java, we know that a ZonedDateTime has a ZoneId (TimeZoneRegion in brick/date-time language), and we know that an OffsetDateTime has a ZoneOffset, but not a ZoneId. In brick/date-time there is no distinction; ZonedDateTime is less specific and ZonedDateTime::getTimeZone can return either a TimeZoneRegion or a TimeZoneOffset. This means we have no way of specifying that a date time shall/must have a TimeZoneRegion.

Having said that, I can think of advantages to the approach you've taken; for example, if we have a case where we want to allow either a ZonedDateTime or an OffsetDateTime (in java speak), without discarding a TimeZoneRegion when one is available, then your approach probably works better.