Re: [PHP-DEV] [PR] DateTime::createFromImmutable() method
On Mon, Mar 9, 2015 at 5:35 AM, Derick Rethans der...@php.net wrote: On Thu, 5 Mar 2015, Trevor Suarez wrote: Good morning internals! I would like to propose a small addition be made to the DateTime class. https://github.com/php/php-src/pull/1145 When the factory method was added, we had this same discussion. And the discussions lead to the conclusion that it does not make sense to have this createFromImmutable factory method on DateTime. Basically with the same reasons that people should type hint against the Interface instead. If you have a method that needs to modify the DateTime, then you must choose between DateTime or DateTimeImmutable. (The interface is read only.) That is, I think it is poor practice to repeat the following over and over in every method: function changeTheDate(DateTimeInterface $dt) { $immutable = new DateTimeImmutable($dt-format('c')); $immutable-setTimeZone(...); // etc return $someCalculation; } That function really needs a DateTimeImmutable, so the burden of converting it ought to be on the caller. It might be able to optimize the number of conversions, etc. I almost always use DateTimeImmutables ... fewer side effects, etc. However, many libraries and code I interface with require a DateTime for such things. (Yes, perhaps they just incorrectly type hint for a DateTime, but I cannot help that.) So I need to convert from DateTimeImmutable to DateTime. e.g., function foo(DateTime $dt) { } $dt = new DateTimeImmutable; foo(new DateTime($dt-format('c')); // the method I use foo(new DateTime($dt)); // doesn't work foo(DateTime::fromInstance($dt)); // doesn't exist (It's possible that DateTimeImmutable::fromInstance($dateTimeImmutable) could just return the same object.) So while not a pressing issue, I do think a more straightforward way of converting would be good because the reality is that you're going to need to do the conversions at some point if you interface with other people's code, no matter how hard you try to use good practices. -- Matthew Leverton -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PR] DateTime::createFromImmutable() method
On Thu, 5 Mar 2015, Trevor Suarez wrote: Good morning internals! I would like to propose a small addition be made to the DateTime class. https://github.com/php/php-src/pull/1145 When the factory method was added, we had this same discussion. And the discussions lead to the conclusion that it does not make sense to have this createFromImmutable factory method on DateTime. Basically with the same reasons that people should type hint against the Interface instead. To emulate that in PHP 5.5 and lower, users can just do: class DateTimeInterface extends DateTime {} I know, that's a bit of a hack. I am against this addition, even though the patch looks OK. cheers, Derick -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PR] DateTime::createFromImmutable() method
Hi Trevor, I had no look to the code as I am no C or internals guru. I just wanted to add that I would prefer a generic DateTimeInterface::createFromInstance(DateTimeInterface $copyFrom) method, so you do not need to distinguish between DateTime and DateTimeImmutable when creating a new instance. That should have be done for the DateTimeImmutable, too. Otherwise you have to do the following: function doSomethingWithNewDate(\DateTimeInterface $date) { if ($date instanceof \DateTimeImmutable) { $newDate = \DateTime::createFromImmutable($date); } else { $newDate = clone $date; } } But if you want to create a new instance from a date, it should not matter if the source date is mutable or immutable. Thanks Dennis Am 05.03.2015 um 08:50 schrieb Trevor Suarez: Good morning internals! I would like to propose a small addition be made to the DateTime class. https://github.com/php/php-src/pull/1145 This is my first contribution to PHP's core, beyond documentation edits. I'm not sure on the formalities that need to be taken for something like this to be considered as an addition, or if an email like this is even appropriate, so I'd absolutely welcome any guidance here. I just saw a possible opportunity for an addition and thought it would be fun to try and contribute to PHP. :) I'm not a C programmer by any means, so please forgive me if I've made any noob mistakes here, haha. Thank you all for your time and consideration! - Trevor -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [PR] DateTime::createFromImmutable() method
Good morning internals! I would like to propose a small addition be made to the DateTime class. https://github.com/php/php-src/pull/1145 This is my first contribution to PHP's core, beyond documentation edits. I'm not sure on the formalities that need to be taken for something like this to be considered as an addition, or if an email like this is even appropriate, so I'd absolutely welcome any guidance here. I just saw a possible opportunity for an addition and thought it would be fun to try and contribute to PHP. :) I'm not a C programmer by any means, so please forgive me if I've made any noob mistakes here, haha. Thank you all for your time and consideration! - Trevor