Re: DateTime microseconds discussion

On 11/08/2016 02:49 PM, Derick Rethans wrote:

Hi, On Tue, 8 Nov 2016, Arjen Schol wrote:

I think you make some bad assumptions here.

Please don't top-reply on this list.

The examples provided by Sjon are scripts submitted to 3v4l.org They may have bad assumptions, but are real life examples of DateTime usage. And they will break.

They already could break. As Dan wrote better: Having bugs happen more frequently is a good thing. It stops you from ignoring edge cases and programming by coincidence.

We have two issues in our codebase. We ARE testing RC release, I think feedback should be taken seriously.

Taking things seriously does not mean having to agree.

Taking things seriously means reading the complete message and not focussing on a detail of an example (https://3v4l.org/YUhFF faces the same problem without constructor argument; e.g. not easy to set microseconds to 0). I've never said we were facing new DateTime == new DateTime problems, but Dan gives a lesson on bad assumptions... No comment on not being able to set microtime to zero.

1. We overloaded DateTime::setTime, this needed an update because of the extra parameter (point 3). BC break.

PHP throws a warning here, but the expected result of the code execusion is correct: [PHP: 7.1.0-dev ] derick@whisky:/tmp $ cat doo.php <?php class MyDate extends DateTimeImmutable { function setTime( $h, $i, $s ) { return parent::setTime( $h, $i + 5, $s ); } } $a = new MyDate(); $b = $a->setTime( 2, 5, 10 ); var_dump( $a, $b ); ?> [PHP: 7.1.0-dev ] derick@whisky:/tmp $ php -n doo.php Warning: Declaration of MyDate::setTime($h, $i, $s) should be compatible with DateTimeImmutable::setTime($hour, $minute, $second = NULL, $microseconds = NULL) in /tmp/doo.php on line 9 object(MyDate)#1 (3) { ["date"]=> string(26) "2016-11-08 13:30:06.371428" ["timezone_type"]=> int(3) ["timezone"]=> string(3) "UTC" } object(MyDate)#2 (3) { ["date"]=> string(26) "2016-11-08 02:10:10.000000" ["timezone_type"]=> int(3) ["timezone"]=> string(3) "UTC" } This is at *most* a BC break because it shows a warning. The code itself isn't broken. In PHP 5.6, this was a E_STRICT warning, which got converted to E_WARNING in PHP 7.0: [PHP: 5.6.28-dev ] derick@whisky:~/dev/php/php-src.git $ php -n -derror_reporting=-1 -ddate.timezone=UTC /tmp/doo.php Strict Standards: Declaration of MyDate::setTime() should be compatible with DateTimeImmutable::setTime($hour, $minute, $second = NULL) in /tmp/doo.php on line 9 … [PHP: 7.0.13-dev ] derick@whisky:~/dev/php/php-src.git $ php -n -derror_reporting=-1 -ddate.timezone=UTC /tmp/doo.php Warning: Declaration of MyDate::setTime($h, $i, $s) should be compatible with DateTimeImmutable::setTime($hour, $minute, $second = NULL) in /tmp/doo.php on line 9 … As this specific overloading does *not* violate the Liskov Substitution Principle, I would argue that it is this *warning* that is the BC break in PHP 7.0, and not the addition of the extra argument-with-default-value.

I think it DOES violate (and hence the warning) LSP. If you DO use the 4th microseconds argument in your application, you cannot replace a DateTime object with CustomDate which does not have this 4th argument, even tough DateTime has a default value for it. If you do want to save a time with microseconds by setTime, you end up without microseconds if you replace DateTime with CustomDate. "[..] if S is a subtype of T, then objects of type T may be replaced with objects of type S (i.e., an object of the type T may be substituted with its subtype object of the type S) without altering any of the desirable properties of that program (correctness, task performed, etc.)"

2. We test the following: A. $date = new DateTime; B. Store in database. C. Retrieve datetime. D. $fromDatabase = new DateTime(storedDateTime); E. $fromDatabase == $date (which fails now, because $date has microsecond precision while $fromDatabase doesn't).

If you don't store microseconds in the database, then I expect that not to work. It's like if you wouldn't store seconds in the database, that you wouldn't get the same result either.

This used to work for 7 years, and now it's not. If I hadn't stored seconds, it wouldn't have worked for 7 years.

Where does this have a bad assumption? We tested an assumption (DateTime has seconds precision), and now it fails. Is that a bad assumption? I think it's just backward breaking with no good way to fix the code.

The bad assumption is, that DateTime has always had microseconds precision, and the bug that got fixes was, that they did not always get correctly initialised:

First, it was virtually impossible to create a DateTime object with the current time WITH microseconds set (DateTime::createFromFormat('U.u', sprintf('%.f', microtime(true))); only method AFAIK) and now it is cumbersome to create a DateTime object without microseconds initialized/set to 0: DateTime::createFromFormat('U', time()); or new DateTime('@' . time()); or afterwards by calling setTime()... This has been the case since 5.3.1.. You can't seriously argue DateTime always had microsecond precision and fixing the initialization took almost 7 years.. DateTime had some obscure way to initialize it with the current time including microseconds, the default __construct() did not show a sign of microsecond suppport. Changing this behaviour 7 years after introduction cannot be called a bugfix (is this going into 5.6 and 7.0 as well?), it's just a feature. Did you ever Googled "microsecond site:php.net/manual"? Only the documentation (comments not included) about createFromFormat mentions 'microsecond'. Making the assumption DateTime supports microtime would only be based on internal, non-documented details. THAT would be a bad assumption. See https://3v4l.org/aCRU0 and https://3v4l.org/4Oid8

[PHP: 7.0.13-dev ] derick@whisky:~/dev/php/php-src.git $ cat doo2.php <?php $date = new DateTimeImmutable( "13:39:04.123456" ); var_dump( $date ); ?> [PHP: 7.0.13-dev ] derick@whisky:~/dev/php/php-src.git $ php -n doo2.php object(DateTimeImmutable)#1 (3) { ["date"]=> string(26) "2016-11-08 13:39:04.123456" ["timezone_type"]=> int(3) ["timezone"]=> string(3) "UTC" }

We could set microseconds to 0, which is cumbersome because of a missing setMicroseconds method. One needs to call setTime with retrieved hours, minutes, seconds or setTimestamp($dt->format('U'))...

As you're overloading the DateTime class anyway, that's merely a minor inconvenience. cheers, Derick