Re: [PHP-DEV] Add __toString() to DateInterval

2021-03-18 Thread Moritz Friedrich
> Would a reasonable way forward be to have such a named method, and have it 
> throw an exception if the instance is not representable in that form?
> 
> That way, you'd be guaranteed that new 
> DateInterval($period->toIsoPeriodString()) would result in an equivalent 
> object, rather than discarding special relative values.

Yep, this sounds like a good idea. Derick, would this work for you?


Regards,
Moritz
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] Add __toString() to DateInterval

2021-03-16 Thread Moritz Friedrich


> I already showed you that: "next weekday". It can either be +1 days, +2 
> days, or +3 days.
…depending on the time of execution, yes. The moment a developer consciously 
decides to transform an interval to an ISO string representation, they 
obviously accept the premise of anchoring the dynamic expression to the current 
point in time.

> Maybe, but the DateInterval class does not represent a ISO interval 
> specification, it represents a timelib interval definition, which you 
> can *also* create from an ISO 8601 interval specification.
Isn’t it the other way around? I’d argue that a DateInterval represents an ISO 
interval specification according to its constructor, which accepts ISO interval 
specifications exclusively, but is *also* capable of creating an instance from 
a timelib interval definition using the static helper method 
`createFromDateString`. From a userland perspective it doesn’t matter how 
timelib handles this internally, as long as the public API stays consistent.

> Converting to just a simple string is not a serialisation, as it loses 
> information. If you care about serialisation, then you need to come up 
> with a way how to do that for every interval that a DateInterval can 
> represent. PHP's "serialize" can already serialise a DateInterval.
…depending on whether you perceive the start and end dates of an interval as an 
inherent part of the interval or external constraints. Just because timelib 
makes the assumption of the latter doesn’t mean the rest of the world shares 
this perspective, as emphasized by the fact of the ISO specification not doing 
so.

Again, I’m not attached to `__toString()`. Having another helper method or a 
special formatting constant, as suggested by others in this thread, would 
likewise be a viable option. I’d merely prefer to have a built-in way of 
parsing *and* composing time intervals as per the ISO specification, making the 
API consistent and easing communication with non-PHP systems. 


Regards,
Moritz
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] Add __toString() to DateInterval

2021-03-16 Thread Rowan Tommins

On 16/03/2021 18:56, Moritz Friedrich wrote:

Again, I’m not attached to `__toString()`. Having another helper method or a 
special formatting constant, as suggested by others in this thread, would 
likewise be a viable option. I’d merely prefer to have a built-in way of 
parsing*and*  composing time intervals as per the ISO specification, making the 
API consistent and easing communication with non-PHP systems.



Would a reasonable way forward be to have such a named method, and have 
it throw an exception if the instance is not representable in that form?


That way, you'd be guaranteed that new 
DateInterval($period->toIsoPeriodString()) would result in an equivalent 
object, rather than discarding special relative values.


Regards,

--
Rowan Tommins
[IMSoP]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] Add __toString() to DateInterval

2021-03-16 Thread Moritz Friedrich


> I also don't understand what you're trying to say. You can't round trip 
> every interval as encoded in a DateInterval object through an ISO 
> interval string, so we shouldn't attempt to do that.


I think you do know what I’m trying to say, but choose to disagree with me. ISO 
interval strings allow to express arbitrary intervals of time as a string, so I 
don’t know what possible intervals you’re referring to that purportedly cannot 
be encoded as an interval string.
If this still revolves around relative intervals, then I don’t see your point 
either. The ISO interval specification is the most appropriate serialization 
format for time intervals as it is commonly used and well understood. By 
choosing to serialize something like „next week“ to an ISO interval, I’m 
willingly accepting the conversion to an absolute amount of time. If the 
specific start and end dates are relevant for the use case, an interval is not 
the appropriate tool, but keeping both dates is.
Having a constructor that only accepts an ISO interval but no way to serialize 
the duration object back to an interval seems just like an inconsistent API to 
me. I’m not particularly attached to using `__toString()`, something like 
`toIsoInterval` or similar would do just as well. The only thing that annoys me 
here is a one-way parser for interval specs, deferring the serialization to 
userland PHP. 


Regards,
Moritz
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] Add __toString() to DateInterval

2021-03-16 Thread Derick Rethans
On Tue, 16 Mar 2021, Moritz Friedrich wrote:

> > I also don't understand what you're trying to say. You can't round 
> > trip every interval as encoded in a DateInterval object through an 
> > ISO interval string, so we shouldn't attempt to do that.
> 
> I think you do know what I’m trying to say, but choose to disagree 
> with me. ISO interval strings allow to express arbitrary intervals of 
> time as a string, so I don’t know what possible intervals you’re 
> referring to that purportedly cannot be encoded as an interval string.

I already showed you that: "next weekday". It can either be +1 days, +2 
days, or +3 days.

> If this still revolves around relative intervals, then I don’t see 
> your point either. The ISO interval specification is the most 
> appropriate serialization format for time intervals as it is commonly 
> used and well understood. 

Maybe, but the DateInterval class does not represent a ISO interval 
specification, it represents a timelib interval definition, which you 
can *also* create from an ISO 8601 interval specification.

> By choosing to serialize something like „next week“ to an ISO 
> interval, I’m willingly accepting the conversion to an absolute amount 
> of time. If the specific start and end dates are relevant for the use 
> case, an interval is not the appropriate tool, but keeping both dates 
> is.

That is not what a DateInterval object represents — instead it 
represents a definition how to get from one DateTime to another one.
And that can not always be represented by an absolute amount.

> Having a constructor that only accepts an ISO interval but no way to 
> serialize the duration object back to an interval seems just like an 
> inconsistent API to me. I’m not particularly attached to using 
> `__toString()`, something like `toIsoInterval` or similar would do 
> just as well. The only thing that annoys me here is a one-way parser 
> for interval specs, deferring the serialization to userland PHP.

Converting to just a simple string is not a serialisation, as it loses 
information. If you care about serialisation, then you need to come up 
with a way how to do that for every interval that a DateInterval can 
represent. PHP's "serialize" can already serialise a DateInterval.

cheers,
Derick

-- 
PHP 7.4 Release Manager
Host of PHP Internals News: https://phpinternals.news
Like Xdebug? Consider supporting me: https://xdebug.org/support
https://derickrethans.nl | https://xdebug.org | https://dram.io
twitter: @derickr and @xdebug

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Add __toString() to DateInterval

2021-03-16 Thread Derick Rethans
On Mon, 15 Mar 2021, Moritz Friedrich wrote:

> I would assume the moment I call some kind of ISO conversion or 
> serialization method on a dynamic interval, I’d receive output 
> relative to now. `createFromDateString` creates the interval relative 
> to now, so why shouldn’t the reverse hold true?

Please don't post top replies.

I also don't understand what you're trying to say. You can't round trip 
every interval as encoded in a DateInterval object through an ISO 
interval string, so we shouldn't attempt to do that.

cheers,
Derick


-- 
PHP 7.4 Release Manager
Host of PHP Internals News: https://phpinternals.news
Like Xdebug? Consider supporting me: https://xdebug.org/support
https://derickrethans.nl | https://xdebug.org | https://dram.io
twitter: @derickr and @xdebug

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Add __toString() to DateInterval

2021-03-15 Thread Moritz Friedrich
I would assume the moment I call some kind of ISO conversion or serialization 
method on a dynamic interval, I’d receive output relative to now. 
`createFromDateString` creates the interval relative to now, so why shouldn’t 
the reverse hold true?

Regards,
Moritz

> Am 15.03.2021 um 10:18 schrieb Derick Rethans :
> 
> On Wed, 3 Mar 2021, Moritz Friedrich wrote:
> 
>> I would like to propose adding a `__toString()` method to the 
>> `DateInterval` class that should return a valid ISO8601 interval 
>> (https://en.wikipedia.org/wiki/ISO_8601#Time_intervals). As it stands, 
>> the class supports creating instances from such interval strings 
>> passed to its constructor, but the reverse isn’t true: The only way to 
>> build a string representation of the interval is by querying the 
>> `DateInterval::format` method for the individual durations multiple 
>> times (see 
>> https://stackoverflow.com/questions/33787039/format-dateinterval-as-iso8601#answers
>>  
>> for examples).
> 
> The DateInterval class can also contain intervals that can't be 
> represented with such an ISO string. For example, how are you going to 
> do:
> 
> $dt = DateInterval::createFromDateString( "next weekday" );
> 
> or
> 
> $dt = DateInterval::createFromDateString( 'next Monday 02:00' );
> 
> __toString needs to work in every case, and I don't think you can do 
> that with ISO 8601 interval strings.
> 
> cheers,
> Derick
> 
> -- 
> PHP 7.4 Release Manager
> Host of PHP Internals News: https://phpinternals.news
> Like Xdebug? Consider supporting me: https://xdebug.org/support
> https://derickrethans.nl | https://xdebug.org | https://dram.io
> twitter: @derickr and @xdebug

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] Add __toString() to DateInterval

2021-03-15 Thread Derick Rethans
On Wed, 3 Mar 2021, Moritz Friedrich wrote:

> I would like to propose adding a `__toString()` method to the 
> `DateInterval` class that should return a valid ISO8601 interval 
> (https://en.wikipedia.org/wiki/ISO_8601#Time_intervals). As it stands, 
> the class supports creating instances from such interval strings 
> passed to its constructor, but the reverse isn’t true: The only way to 
> build a string representation of the interval is by querying the 
> `DateInterval::format` method for the individual durations multiple 
> times (see 
> https://stackoverflow.com/questions/33787039/format-dateinterval-as-iso8601#answers
>  
> for examples).

The DateInterval class can also contain intervals that can't be 
represented with such an ISO string. For example, how are you going to 
do:

$dt = DateInterval::createFromDateString( "next weekday" );

or

$dt = DateInterval::createFromDateString( 'next Monday 02:00' );

__toString needs to work in every case, and I don't think you can do 
that with ISO 8601 interval strings.

cheers,
Derick

-- 
PHP 7.4 Release Manager
Host of PHP Internals News: https://phpinternals.news
Like Xdebug? Consider supporting me: https://xdebug.org/support
https://derickrethans.nl | https://xdebug.org | https://dram.io
twitter: @derickr and @xdebug

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Add __toString() to DateInterval

2021-03-03 Thread Moritz Friedrich



> Am 03.03.2021 um 14:01 schrieb Andreas Heigl :
> 
> I'd rather see those classes as ValueObjects that should not have to
> take care about their external representation. And a custom Formatter
> that handles all the weird edge cases as a separate entity would be a
> much easier to maintain approach. And such a Formatter can easily be
> build in userland (I think I wrote one myself at one point) and so the
> maintenance-burden would also not be added to internals.
> 
> That would also apply to the DateTimeInterval::format() method but that
> would mean a massive BC break so it is most likely out of the question.
> Nevertheless I would prefer an external library to handle all those
> formatting issues and treat the DateTime lib as internal ValueObjects

I’d like to respectfully disagree. If we were to go down the ValueObject route, 
DateTime/DateInterval should not be able to create new instances from formatted 
strings in the first place. PHP Date classes have always been intertwined with 
their output formatting however, so this is how the ecosystem uses them. In 
that sense, I’d expect DateInterval to be able to return the format it was 
instantiated with, but it isn't.
The PHP API may have its warts, but I prefer my warts consistent.

Regards 
Moritz
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] Add __toString() to DateInterval

2021-03-03 Thread Chase Peeler
On Wed, Mar 3, 2021 at 8:42 AM Andreas Heigl  wrote:

>
> Am 03.03.21 um 14:24 schrieb Moritz Friedrich:
> >
> >
> >> Am 03.03.2021 um 14:01 schrieb Andreas Heigl :
> >>
> >> I'd rather see those classes as ValueObjects that should not have to
> >> take care about their external representation. And a custom Formatter
> >> that handles all the weird edge cases as a separate entity would be a
> >> much easier to maintain approach. And such a Formatter can easily be
> >> build in userland (I think I wrote one myself at one point) and so the
> >> maintenance-burden would also not be added to internals.
> >>
> >> That would also apply to the DateTimeInterval::format() method but that
> >> would mean a massive BC break so it is most likely out of the question.
> >> Nevertheless I would prefer an external library to handle all those
> >> formatting issues and treat the DateTime lib as internal ValueObjects
> >
> > I’d like to respectfully disagree. If we were to go down the ValueObject
> route, DateTime/DateInterval should not be able to create new instances
> from formatted strings in the first place. PHP Date classes have always
> been intertwined with their output formatting however, so this is how the
> ecosystem uses them. In that sense, I’d expect DateInterval to be able to
> return the format it was instantiated with, but it isn't.
>
> Well. ValueObjects should be able to create new Instances of themselves
> via factory methods. But that would mean a lot of tweaking at perhaps no
> added benefit.
>
> But TBH: DateTimeImmutable:fromString() instead of new
> DateTimeImmutable() would be awesome... Or even better:
> DateTime::fromString() with DateTimeImmutable as return type
>
>
> > The PHP API may have its warts, but I prefer my warts consistent.
>
> I feel you.
>
> As the other Objects have a format() method why not have a format method
> for DatePeriod as well? It does not need to take a parameter as there
> are no different formats (at least that I could think of).
>
> But I would rather not add more magic (methods) to PHP...
>
>
I wouldn't call utilizing __toString() adding more magic methods to PHP.
It's utilizing a magic method that already exists. I'd also argue that of
all the magic methods, __toString() is probably the least likely to cause
issues.
1.) Core/bundled extensions don't usually have major documentation issues
2.) Even if __toString() functionality isn't properly documented, I'm
guessing (please prove me wrong if I am) that working with string output of
an object that hasn't implented __toString() has hardly any use in
production (not counting tests). As such, if __toString() were to magically
appear (pun intended) the side effects would be minimal.
3.) Unlike some of the other magic methods (__get, __set, __call for
instance) __toString() is basically implementing an interface from a user
perspective.

I personally like the idea of having more explicit methods and then
allowing __toString() to invoke one of those.

I don't know if I like this idea or not, but, just to throw it out there,
when dealing with something like DateTime, you could even support the
ability to set a "default" format that would be utilized by __toString
(with the default format having a default value if not set)

$dt = new \DateTime();
echo $dt; //outputs 2021-01-01T03:15:00-05:00
$dt->setDefaultFormat("Ymdhis");
echo $dt; //outputs 20210101031500

Considering that when I'm doing something and need frequent string
representations of a DateTime that are always the same format, I usually do
$dt = new \DateTime();
$format = "Ymdhis";
echo $dt->format($format);
$s = $dt->format($format);

The only thing you really risk with the setDefaultFormat() option is that
you pass the DateTime to another method and they aren't really sure what is
there and/or override your default format. I think as long as it's well
documented, that could be easy to work around.
function foo(\DateTime $dt){
   $currentFormat = $dt->getDefaultFormat();
   $dt->setDefaultFormat("m/d/Y");
   //stuff
   $dt->setDefaultFormat($currentFormat);
}

Again, I'm not sold on such functionality, just throwing out some ideas to
see what others thing.



> Cheers
>
> Andreas
>
> --
>   ,,,
>  (o o)
> +-ooO-(_)-Ooo-+
> | Andreas Heigl   |
> | mailto:andr...@heigl.org  N 50°22'59.5" E 08°23'58" |
> | https://andreas.heigl.org   |
> +-+
> | https://hei.gl/appointmentwithandreas   |
> +-+
>
>

-- 
Chase Peeler
chasepee...@gmail.com


Re: [PHP-DEV] Add __toString() to DateInterval

2021-03-03 Thread Moritz Friedrich
Am 03.03.2021 um 11:53 schrieb Pierre :
> 
> Le 03/03/2021 à 11:37, Moritz Friedrich a écrit :
>> Hi internals,
>> I’ve been active in the PHP ecosystem as @Radiergummi for quite a while now, 
>> but not on internals yet, so: nice to meet you all!
>> 
>> I would like to propose adding a `__toString()` method to the `DateInterval` 
>> class that should return a valid ISO8601 interval 
>> (https://en.wikipedia.org/wiki/ISO_8601#Time_intervals). As it stands, the 
>> class supports creating instances from such interval strings passed to its 
>> constructor, but the reverse isn’t true: The only way to build a string 
>> representation of the interval is by querying the `DateInterval::format` 
>> method for the individual durations multiple times (see 
>> https://stackoverflow.com/questions/33787039/format-dateinterval-as-iso8601#answers
>>  for examples).
>> 
>> Allowing to cast `DateInterval`s to strings would make the most sense in my 
>> opinion, and probably doesn’t break BC. Use cases include communicating with 
>> other APIs that work with ISO8601, or persistence of periods in databases, 
>> for example.
>> 
>> So instead of the following:
>> 
>> function formatDateInterval(DateInterval $interval, string $default = 
>> 'PT0S'): string {
>> return rtrim(str_replace(
>> ['M0S', 'H0M', 'DT0H', 'M0D', 'P0Y', 'Y0M', 'P0M'],
>> ['M', 'H', 'DT', 'M', 'P', 'Y', 'P'],
>> $interval->format('P%yY%mM%dDT%hH%iM%sS')
>> ), 'PT') ?: $default;
>> }
>>  formatDateInterval(new DateInterval('P1DT5H')) === 'P1DT5H'
>> 
>> I’d like to be able to do this:
>> 
>> (string)new DateInterval('P1DT5H') === 'P1DT5H'
>> 
>> Following alternative approaches come to mind:
>> 
>> 1. Adding a new, specialized method to the class (eg. 
>> `DateInterval::toIsoString(): string`):
>>While being the most BC-safe option, this would be completely different 
>> to the rest of the PHP date APIs.
>> 
>> 2. Adding a new interval format constant to be passed to 
>> `DateInterval::format` (eg. `DATE_INTERVAL_ISO8601`):
>>Most in line with the `DateTime` API, but probably requires special case 
>> handling in `format` code.
>> 
>> From those, having `__toString` seems most desirable to me.
>> While I’m not proficient with C, I’d really like to see this implemented and 
>> would gladly help out where I can. Might someone be able to advise?
>> 
>> 
>> Sincerely
>> Moritz / Radiergummi
> 
> Hello,
> 
> I don't like implicit __toString() methods, they have to be very carefully 
> documented and it's "magic". Wouldn't it be best to have both the 
> `toIsoString()` and `__toString()` (as a fallback) methods ?
> 
> Having the constant is a good idea too, it would make it probably more 
> consistent with the DateTime class, I'm not opposed having three variations 
> to do skin the same cat.
> 
> Regards,
> 
> Pierre
> 
> -- 
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
> 

Hi Pierre,

Having `__toString` proxy to `toIsoString` internally sounds like a good idea, 
but I’m not too fond of the constant - all other built-in date constants 
translate to a string of plain format characters, which isn’t possible in this 
case, so it would be meaningless everywhere else.

Sincerely,
Moritz

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



[PHP-DEV] Add __toString() to DateInterval

2021-03-03 Thread Moritz Friedrich
Hi internals,
I’ve been active in the PHP ecosystem as @Radiergummi for quite a while now, 
but not on internals yet, so: nice to meet you all!

I would like to propose adding a `__toString()` method to the `DateInterval` 
class that should return a valid ISO8601 interval 
(https://en.wikipedia.org/wiki/ISO_8601#Time_intervals). As it stands, the 
class supports creating instances from such interval strings passed to its 
constructor, but the reverse isn’t true: The only way to build a string 
representation of the interval is by querying the `DateInterval::format` method 
for the individual durations multiple times (see 
https://stackoverflow.com/questions/33787039/format-dateinterval-as-iso8601#answers
 for examples).

Allowing to cast `DateInterval`s to strings would make the most sense in my 
opinion, and probably doesn’t break BC. Use cases include communicating with 
other APIs that work with ISO8601, or persistence of periods in databases, for 
example. 

So instead of the following:

function formatDateInterval(DateInterval $interval, string $default = 
'PT0S'): string {
return rtrim(str_replace(
['M0S', 'H0M', 'DT0H', 'M0D', 'P0Y', 'Y0M', 'P0M'],
['M', 'H', 'DT', 'M', 'P', 'Y', 'P'],
$interval->format('P%yY%mM%dDT%hH%iM%sS')
), 'PT') ?: $default;
}

formatDateInterval(new DateInterval('P1DT5H')) === 'P1DT5H'

I’d like to be able to do this:

(string)new DateInterval('P1DT5H') === 'P1DT5H'

Following alternative approaches come to mind:

1. Adding a new, specialized method to the class (eg. 
`DateInterval::toIsoString(): string`):  
   While being the most BC-safe option, this would be completely different to 
the rest of the PHP date APIs.

2. Adding a new interval format constant to be passed to `DateInterval::format` 
(eg. `DATE_INTERVAL_ISO8601`):  
   Most in line with the `DateTime` API, but probably requires special case 
handling in `format` code.

From those, having `__toString` seems most desirable to me.  
While I’m not proficient with C, I’d really like to see this implemented and 
would gladly help out where I can. Might someone be able to advise?


Sincerely
Moritz / Radiergummi
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] Add __toString() to DateInterval

2021-03-03 Thread Larry Garfield
On Wed, Mar 3, 2021, at 4:37 AM, Moritz Friedrich wrote:
> Hi internals,
> I’ve been active in the PHP ecosystem as @Radiergummi for quite a while 
> now, but not on internals yet, so: nice to meet you all!
> 
> I would like to propose adding a `__toString()` method to the 
> `DateInterval` class that should return a valid ISO8601 interval 
> (https://en.wikipedia.org/wiki/ISO_8601#Time_intervals). As it stands, 
> the class supports creating instances from such interval strings passed 
> to its constructor, but the reverse isn’t true: The only way to build a 
> string representation of the interval is by querying the 
> `DateInterval::format` method for the individual durations multiple 
> times (see 
> https://stackoverflow.com/questions/33787039/format-dateinterval-as-iso8601#answers
>  for examples).
> 
> Allowing to cast `DateInterval`s to strings would make the most sense 
> in my opinion, and probably doesn’t break BC. Use cases include 
> communicating with other APIs that work with ISO8601, or persistence of 
> periods in databases, for example. 
> 
> So instead of the following:
> 
> function formatDateInterval(DateInterval $interval, string $default 
> = 'PT0S'): string {
> return rtrim(str_replace(
> ['M0S', 'H0M', 'DT0H', 'M0D', 'P0Y', 'Y0M', 'P0M'],
> ['M', 'H', 'DT', 'M', 'P', 'Y', 'P'],
> $interval->format('P%yY%mM%dDT%hH%iM%sS')
> ), 'PT') ?: $default;
> }
> 
> formatDateInterval(new DateInterval('P1DT5H')) === 'P1DT5H'
> 
> I’d like to be able to do this:
> 
> (string)new DateInterval('P1DT5H') === 'P1DT5H'
> 
> Following alternative approaches come to mind:
> 
> 1. Adding a new, specialized method to the class (eg. 
> `DateInterval::toIsoString(): string`):  
>While being the most BC-safe option, this would be completely 
> different to the rest of the PHP date APIs.
> 
> 2. Adding a new interval format constant to be passed to 
> `DateInterval::format` (eg. `DATE_INTERVAL_ISO8601`):  
>Most in line with the `DateTime` API, but probably requires special 
> case handling in `format` code.
> 
> From those, having `__toString` seems most desirable to me.  
> While I’m not proficient with C, I’d really like to see this 
> implemented and would gladly help out where I can. Might someone be 
> able to advise?

I agree that easier round-tripping of interval data is a good thing.  As for 
which approach to take, I think we should defer to Derick, or at least see what 
his input is. :-)  I don't have a super strong preference between these 3 
options, other than not liking the idea of tossing that off to user space.  For 
better or worse DateTime is a core functionality and API, so it should be 
implemented in one place.  Beyond that, whatever Derick feels is most 
consistent with the rest of the API is OK by me.

Let's not bikeshed this into a larger overhaul of the whole DateTime API. :-)

--Larry Garfield

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] Add __toString() to DateInterval

2021-03-03 Thread Andreas Heigl

Am 03.03.21 um 14:24 schrieb Moritz Friedrich:
> 
> 
>> Am 03.03.2021 um 14:01 schrieb Andreas Heigl :
>>
>> I'd rather see those classes as ValueObjects that should not have to
>> take care about their external representation. And a custom Formatter
>> that handles all the weird edge cases as a separate entity would be a
>> much easier to maintain approach. And such a Formatter can easily be
>> build in userland (I think I wrote one myself at one point) and so the
>> maintenance-burden would also not be added to internals.
>>
>> That would also apply to the DateTimeInterval::format() method but that
>> would mean a massive BC break so it is most likely out of the question.
>> Nevertheless I would prefer an external library to handle all those
>> formatting issues and treat the DateTime lib as internal ValueObjects
> 
> I’d like to respectfully disagree. If we were to go down the ValueObject 
> route, DateTime/DateInterval should not be able to create new instances from 
> formatted strings in the first place. PHP Date classes have always been 
> intertwined with their output formatting however, so this is how the 
> ecosystem uses them. In that sense, I’d expect DateInterval to be able to 
> return the format it was instantiated with, but it isn't.

Well. ValueObjects should be able to create new Instances of themselves
via factory methods. But that would mean a lot of tweaking at perhaps no
added benefit.

But TBH: DateTimeImmutable:fromString() instead of new
DateTimeImmutable() would be awesome... Or even better:
DateTime::fromString() with DateTimeImmutable as return type


> The PHP API may have its warts, but I prefer my warts consistent.

I feel you.

As the other Objects have a format() method why not have a format method
for DatePeriod as well? It does not need to take a parameter as there
are no different formats (at least that I could think of).

But I would rather not add more magic (methods) to PHP...

Cheers

Andreas

-- 
  ,,,
 (o o)
+-ooO-(_)-Ooo-+
| Andreas Heigl   |
| mailto:andr...@heigl.org  N 50°22'59.5" E 08°23'58" |
| https://andreas.heigl.org   |
+-+
| https://hei.gl/appointmentwithandreas   |
+-+



OpenPGP_signature
Description: OpenPGP digital signature


Re: [PHP-DEV] Add __toString() to DateInterval

2021-03-03 Thread Andreas Heigl
Hey

Am 03.03.21 um 13:05 schrieb Hans Henrik Bergan:
> if FWIW if DateTime::__toString() was just
> function __toString():string{
> return $this->format(\DateTime::RFC3339);
> }
> 
> i certainly wouldn't complain.
> 
> On Wed, 3 Mar 2021 at 12:16, Bruce Weirdan  wrote:
> 
>> On Wed, Mar 3, 2021 at 1:07 PM Moritz Friedrich  wrote:
>>
>>> but I’m not too fond of the constant - all other built-in date constants
>>> translate to a string of plain format characters, which isn’t possible in
>>> this case
>>
>>
>> Adding another format character (similar to %c used by
>> DateTimeInterface::format()) would solve that.

I'd rather see those classes as ValueObjects that should not have to
take care about their external representation. And a custom Formatter
that handles all the weird edge cases as a separate entity would be a
much easier to maintain approach. And such a Formatter can easily be
build in userland (I think I wrote one myself at one point) and so the
maintenance-burden would also not be added to internals.

That would also apply to the DateTimeInterval::format() method but that
would mean a massive BC break so it is most likely out of the question.
Nevertheless I would prefer an external library to handle all those
formatting issues and treat the DateTime lib as internal ValueObjects

Cheers

Andreas

-- 
  ,,,
 (o o)
+-ooO-(_)-Ooo-+
| Andreas Heigl   |
| mailto:andr...@heigl.org  N 50°22'59.5" E 08°23'58" |
| https://andreas.heigl.org   |
+-+
| https://hei.gl/appointmentwithandreas   |
+-+



OpenPGP_signature
Description: OpenPGP digital signature


Re: [PHP-DEV] Add __toString() to DateInterval

2021-03-03 Thread Pierre

Le 03/03/2021 à 13:05, Hans Henrik Bergan a écrit :

if FWIW if DateTime::__toString() was just
function __toString():string{
 return $this->format(\DateTime::RFC3339);
}

i certainly wouldn't complain.

On Wed, 3 Mar 2021 at 12:16, Bruce Weirdan  wrote:


Considering the replies, PHP Date(Time|Interval) API could probably born 
again from a nice cleanup/improvements iteration.


The one thing I miss the most is not having a Date (especially for when 
you fetch dates from SQL, and you don't care about TZ handling, you may 
very bad surprises using a DateTime to reprensent it on the PHP side due 
to potential implicit TZ conversion, which may create +1 or -1 decal in 
your dates, either at instance creation or display), along with a Time 
type, and DateTime being an aggregate of the two altogether.


--

Pierre

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] Add __toString() to DateInterval

2021-03-03 Thread Hans Henrik Bergan
if FWIW if DateTime::__toString() was just
function __toString():string{
return $this->format(\DateTime::RFC3339);
}

i certainly wouldn't complain.

On Wed, 3 Mar 2021 at 12:16, Bruce Weirdan  wrote:

> On Wed, Mar 3, 2021 at 1:07 PM Moritz Friedrich  wrote:
>
> > but I’m not too fond of the constant - all other built-in date constants
> > translate to a string of plain format characters, which isn’t possible in
> > this case
>
>
> Adding another format character (similar to %c used by
> DateTimeInterface::format()) would solve that.
>
> --
>   Best regards,
>   Bruce Weirdan mailto:
> weir...@gmail.com
>


Re: [PHP-DEV] Add __toString() to DateInterval

2021-03-03 Thread Bruce Weirdan
On Wed, Mar 3, 2021 at 1:07 PM Moritz Friedrich  wrote:

> but I’m not too fond of the constant - all other built-in date constants
> translate to a string of plain format characters, which isn’t possible in
> this case


Adding another format character (similar to %c used by
DateTimeInterface::format()) would solve that.

-- 
  Best regards,
  Bruce Weirdan mailto:
weir...@gmail.com


Re: [PHP-DEV] Add __toString() to DateInterval

2021-03-03 Thread Pierre

Le 03/03/2021 à 11:37, Moritz Friedrich a écrit :

Hi internals,
I’ve been active in the PHP ecosystem as @Radiergummi for quite a while now, 
but not on internals yet, so: nice to meet you all!

I would like to propose adding a `__toString()` method to the `DateInterval` 
class that should return a valid ISO8601 interval 
(https://en.wikipedia.org/wiki/ISO_8601#Time_intervals). As it stands, the 
class supports creating instances from such interval strings passed to its 
constructor, but the reverse isn’t true: The only way to build a string 
representation of the interval is by querying the `DateInterval::format` method 
for the individual durations multiple times (see 
https://stackoverflow.com/questions/33787039/format-dateinterval-as-iso8601#answers
 for examples).

Allowing to cast `DateInterval`s to strings would make the most sense in my 
opinion, and probably doesn’t break BC. Use cases include communicating with 
other APIs that work with ISO8601, or persistence of periods in databases, for 
example.

So instead of the following:

 function formatDateInterval(DateInterval $interval, string $default = 
'PT0S'): string {
 return rtrim(str_replace(
 ['M0S', 'H0M', 'DT0H', 'M0D', 'P0Y', 'Y0M', 'P0M'],
 ['M', 'H', 'DT', 'M', 'P', 'Y', 'P'],
 $interval->format('P%yY%mM%dDT%hH%iM%sS')
 ), 'PT') ?: $default;
 }
 
 formatDateInterval(new DateInterval('P1DT5H')) === 'P1DT5H'


I’d like to be able to do this:

 (string)new DateInterval('P1DT5H') === 'P1DT5H'

Following alternative approaches come to mind:

1. Adding a new, specialized method to the class (eg. 
`DateInterval::toIsoString(): string`):
While being the most BC-safe option, this would be completely different to 
the rest of the PHP date APIs.

2. Adding a new interval format constant to be passed to `DateInterval::format` 
(eg. `DATE_INTERVAL_ISO8601`):
Most in line with the `DateTime` API, but probably requires special case 
handling in `format` code.

 From those, having `__toString` seems most desirable to me.
While I’m not proficient with C, I’d really like to see this implemented and 
would gladly help out where I can. Might someone be able to advise?


Sincerely
Moritz / Radiergummi


Hello,

I don't like implicit __toString() methods, they have to be very 
carefully documented and it's "magic". Wouldn't it be best to have both 
the `toIsoString()` and `__toString()` (as a fallback) methods ?


Having the constant is a good idea too, it would make it probably more 
consistent with the DateTime class, I'm not opposed having three 
variations to do skin the same cat.


Regards,

Pierre

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php