Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:96772
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: unknown (pb1.pair.com: domain php.net does not designate 82.113.146.227 as permitted sender)
Date: Tue, 8 Nov 2016 13:49:17 +0000 (GMT)
To: Arjen Schol <arjen@parse.nl>
cc: PHP Developers Mailing List <internals@lists.php.net>
In-Reply-To: <DB.40.23587.332C1285@pb1.pair.com>
Message-ID: <alpine.DEB.2.20.1611081321150.4870@whisky.home.derickrethans.nl>
References: <46.92.05967.9AB91285@pb1.pair.com> <CA+kxMuR1wVzyAA3RkN2VxmgUNFDm2Z-h02HChFVwT+=imxG4dQ@mail.gmail.com> <DB.40.23587.332C1285@pb1.pair.com>
User-Agent: Alpine 2.20 (DEB 67 2015-01-07)
MIME-Version: 1.0
Content-Type: multipart/mixed; BOUNDARY="8323329-1570356182-1478612957=:4870"
Subject: Re: [PHP-DEV] DateTime microseconds discussion
From: derick@php.net (Derick Rethans)

--8323329-1570356182-1478612957=:4870
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: QUOTED-PRINTABLE

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:

=09> Having bugs happen more frequently is a good thing. It stops=20
=09> 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=20
execusion is correct:

=09[PHP: 7.1.0-dev  ] derick@whisky:/tmp $ cat doo.php=20
=09<?php
=09class MyDate extends DateTimeImmutable
=09{
=09=09function setTime( $h, $i, $s )
=09=09{
=09=09=09return parent::setTime( $h, $i + 5, $s );
=09=09}
=09}

=09$a =3D new MyDate();
=09$b =3D $a->setTime( 2, 5, 10 );

=09var_dump( $a, $b );
=09?>

=09[PHP: 7.1.0-dev  ] derick@whisky:/tmp $ php -n doo.php

=09Warning: Declaration of MyDate::setTime($h, $i, $s) should be compatible=
 with DateTimeImmutable::setTime($hour, $minute, $second =3D NULL, $microse=
conds =3D NULL) in /tmp/doo.php on line 9
=09object(MyDate)#1 (3) {
=09  ["date"]=3D>
=09  string(26) "2016-11-08 13:30:06.371428"
=09  ["timezone_type"]=3D>
=09  int(3)
=09  ["timezone"]=3D>
=09  string(3) "UTC"
=09}
=09object(MyDate)#2 (3) {
=09  ["date"]=3D>
=09  string(26) "2016-11-08 02:10:10.000000"
=09  ["timezone_type"]=3D>
=09  int(3)
=09  ["timezone"]=3D>
=09  string(3) "UTC"
=09}

This is at *most* a BC break because it shows a warning. The code itself is=
n't broken.

In PHP 5.6, this was a E_STRICT warning, which got converted to E_WARNING i=
n PHP 7.0:

=09[PHP: 5.6.28-dev  ] derick@whisky:~/dev/php/php-src.git $ php -n -derror=
_reporting=3D-1 -ddate.timezone=3DUTC /tmp/doo.php=20
=09Strict Standards: Declaration of MyDate::setTime() should be compatible =
with DateTimeImmutable::setTime($hour, $minute, $second =3D NULL) in /tmp/d=
oo.php on line 9
=09=E2=80=A6

=09[PHP: 7.0.13-dev  ] derick@whisky:~/dev/php/php-src.git $ php -n -derror=
_reporting=3D-1 -ddate.timezone=3DUTC /tmp/doo.php=20
=09Warning: Declaration of MyDate::setTime($h, $i, $s) should be compatible=
 with DateTimeImmutable::setTime($hour, $minute, $second =3D NULL) in /tmp/=
doo.php on line 9
=09=E2=80=A6

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:
>=20
> A. $date =3D new DateTime;
> B. Store in database.
> C. Retrieve datetime.
> D. $fromDatabase =3D new DateTime(storedDateTime);
> E. $fromDatabase =3D=3D $date (which fails now, because $date has microse=
cond
> precision while $fromDatabase doesn't).

If you don't store microseconds in the database, then I expect that not=20
to work. It's like if you wouldn't store seconds in the database, that=20
you wouldn't get the same result either.
=20
> 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 i=
t's
> just backward breaking with no good way to fix the code.

The bad assumption is, that DateTime has always had microseconds=20
precision, and the bug that got fixes was, that they did not always get=20
correctly initialised:

=09[PHP: 7.0.13-dev  ] derick@whisky:~/dev/php/php-src.git $ cat doo2.php=
=20
=09<?php
=09$date =3D new DateTimeImmutable( "13:39:04.123456" );
=09var_dump( $date );
=09?>

=09[PHP: 7.0.13-dev  ] derick@whisky:~/dev/php/php-src.git $ php -n doo2.ph=
p
=09object(DateTimeImmutable)#1 (3) {
=09  ["date"]=3D>
=09  string(26) "2016-11-08 13:39:04.123456"
=09  ["timezone_type"]=3D>
=09  int(3)
=09  ["timezone"]=3D>
=09  string(3) "UTC"
=09}

> We could set microseconds to 0, which is cumbersome because of a=20
> missing setMicroseconds method. One needs to call setTime with=20
> retrieved hours, minutes, seconds or setTimestamp($dt->format('U'))...

As you're overloading the DateTime class anyway, that's merely a minor=20
inconvenience.

cheers,
Derick
--8323329-1570356182-1478612957=:4870--