Re: [PHP-DEV] [PR] DateTime::createFromImmutable() method

2015-03-09 Thread Matthew Leverton
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

2015-03-09 Thread Derick Rethans
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

2015-03-05 Thread Dennis Birkholz
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

2015-03-04 Thread 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