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.
> 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.
> 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.
> 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:
[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
