Everything

php

Home » Project » Newsgroups » php.internals » __clone() implementation »

__clone() implementation

php.internals

Stephane Drouard

22 years ago

I'm currently using PHP4 and I'm evaluating PHP5 for its new exciting features.

I have some remarks, split into distinct posts.

The first one is about the current implementation of __clone():

* a derived __clone() can't call parent::__clone() because $that is only declared for the first __clone() call. So a derived __clone() has to include the code of its parent's __clone(). It's not only bad programming, but it can't work when a parent implements private members.
* when a class implements __clone(), all its derived classes have also to implement it to transfer their own members.

There are a several possibilities to get round the first point, but in order to also simplify __clone() implementation, here is a way it could be implemented, close to __construct() implementation:

* the system firstly does a bit for bit copy of the source object,
* then it calls __clone() (which at worst does nothing, if no class implements it),
* __clone() accesses members using $this to handle those (and only those) that need particular actions,
* this is the responsibility of a derived __clone() to call (or not) parent::__clone().

Advantages:
* same behaviour as __construct(), except that $this is already initialized,
* it guaranties that all members will be transfered, by reference for objects, by copy for the others,
* __clone() just needs to be implemented to handle members that need particular actions,
* derived classes do not need to implement __clone() when one of its parents does,
* no more need of $that.

In terms of performance, I don't think this implementation is bad, because a bit for bit copy is certainly faster than an interpreted member-to-member transfer, except if all members have to be handled in a particular way.

I also consider the __clone() implementation not coherent with its calling syntax.
Indeed "$newObj = $obj->__clone();" lets think of an implementation like this:

<?
class Foo {
function __clone() {
$o = new Foo($this->..., $this->...);
$o->...; // eventually
return $o;
}
}
?>

Because __clone() is considered as a copy constructor, it could be implemented in a similar way as for object construction, for example: "$newObj = clone $obj;", "clone" being a language keyword.

Regards,
Stephane

Marcus Börger

22 years ago

Hello Stephane,

you're right $that must be available in the derived __clone().

Thursday, January 22, 2004, 3:37:11 PM, you wrote:

> I'm currently using PHP4 and I'm evaluating PHP5 for its new exciting features.

> I have some remarks, split into distinct posts.

> The first one is about the current implementation of __clone():

> * a derived __clone() can't call parent::__clone() because $that is
> only declared for the first __clone() call. So a derived __clone() has to
> include the code of its parent's __clone(). It's not only bad programming,
> but it can't work when a parent implements private members.
> * when a class implements __clone(), all its derived classes have also
> to implement it to transfer their own members.

> There are a several possibilities to get round the first point, but in
> order to also simplify __clone() implementation, here is a way it could be
> implemented, close to __construct() implementation:

> * the system firstly does a bit for bit copy of the source object,
> * then it calls __clone() (which at worst does nothing, if no class implements it),
> * __clone() accesses members using $this to handle those (and only
> those) that need particular actions,
> * this is the responsibility of a derived __clone() to call (or not) parent::__clone().

> Advantages:
> * same behaviour as __construct(), except that $this is already initialized,
> * it guaranties that all members will be transfered, by reference for objects, by copy for the others,
> * __clone() just needs to be implemented to handle members that need particular actions,
> * derived classes do not need to implement __clone() when one of its parents does,
> * no more need of $that.

> In terms of performance, I don't think this implementation is bad,
> because a bit for bit copy is certainly faster than an interpreted
> member-to-member transfer, except if all members have to be handled in a
> particular way.

> I also consider the __clone() implementation not coherent with its calling syntax.
> Indeed "$newObj = $obj->__clone();" lets think of an implementation like this:

> <?
> class Foo {
>   function __clone() {
>     $o = new Foo($this->..., $this->...);
>     $o->...; // eventually
>     return $o;
>   }
> }

?>>

> Because __clone() is considered as a copy constructor, it could be
> implemented in a similar way as for object construction, for example:
> "$newObj = clone $obj;", "clone" being a language keyword.

> Regards,
> Stephane

--
Best regards,
Marcus                            mailto:helly@php.net

Stephane Drouard

22 years ago

Marcus,

> you're right $that must be available in the derived __clone().

You will solve one problem: being able to call parent::__clone(), but it won't remove the constraint on derived classes (declaring members) to implement __clone() when one of its parents implements it.
This is just to limit risk of bugs, especially when you change a class implementation (not its interface) that now requires __clone() in a class that is the parent of others.

Regards,
Stephane

Unnamed Person

22 years ago

I was told earlier that this is the correct way to do it:

public function __clone()
{
$this = parent::__clone();

$this->... = $that->...;
...
}

Zeev Suraski

22 years ago

Stephane,

Andi and I have revisited the __clone() implementation and must agree that
it wasn't quite right (mainly due to it not working with inheritance).
We have rewritten it now (major change!!!) because we didn't want PHP 5 to
be released with a fundamentally flawed mechanism.

Here's our commit message:

Redesign the clone() feature to fix some fundamental flaws in the previous
implementation.
Using clone directly is now done using
$replica = clone $src;
Clone methods must now be declared as follows:
function __clone($that)
{
}
Clone methods in derived classes can call the __clone method of their parent
classes using parent::__clone($that)

Zeev

At 16:37 22/01/2004, Stephane Drouard wrote:

Stephane Drouard

22 years ago

== Quote from Zeev Suraski (zeev@zend.com)'s article

> Andi and I have revisited the __clone() implementation and must agree that
> it wasn't quite right (mainly due to it not working with inheritance).
> We have rewritten it now (major change!!!) because we didn't want PHP 5 to
> be released with a fundamentally flawed mechanism.
>

Zeev,

Sounds good.

However let me precise some troubles with the new implementation.

1/ Imagine a class with members, but it does not implement __clone because the bit for bit copy is OK. Now you derive this class and you need to implement __clone. Because the first class does not implement it, you can't call parent::__clone (I assume). So this is the responsibility of the derived class to clone the base class. This is not only not clean, but private members won't be visible.

2/ It still does not help what I already mentioned: when a class implements __clone(), all its derived classes have also to implement it to transfer their own members.

For sure, in these 2 cases there are work arounds:
* case 1: modify the base class to implement __clone,
* case 2: implement __clone in the derived classes.

Another idea for case 1 could be to be able to call parent::__clone even if the class does not implement it and have the bit for bit copy (but just for the base class).

Regards,
Stephane

Adam Bregenzer

22 years ago

On Mon, 2004-02-02 at 13:18, Stephane Drouard wrote:

> Another idea for case 1 could be to be able to call parent::__clone
> even if the class does not implement it and have the bit for bit copy
> (but just for the base class).

I also think a default __clone method that does a bit for bit copy would
be a good thing.  What if I do $copy = clone $some_obj; when $some_obj's
class and all its parent classes do not implement __clone?  This could
be important if someone were to write proper php5 code that used classes
written for php4.

--
Adam Bregenzer
adam@bregenzer.net

Andi Gutmans

22 years ago

At 01:26 PM 2/2/2004 -0500, Adam Bregenzer wrote:

>On Mon, 2004-02-02 at 13:18, Stephane Drouard wrote:
> > Another idea for case 1 could be to be able to call parent::__clone
> > even if the class does not implement it and have the bit for bit copy
> > (but just for the base class).
>
>I also think a default __clone method that does a bit for bit copy would
>be a good thing.  What if I do $copy = clone $some_obj; when $some_obj's
>class and all its parent classes do not implement __clone?  This could
>be important if someone were to write proper php5 code that used classes
>written for php4.

That already happens today (or it should).

Andi

Zeev Suraski

22 years ago

At 20:18 02/02/2004, Stephane Drouard wrote:

>== Quote from Zeev Suraski (zeev@zend.com)'s article
> > Andi and I have revisited the __clone() implementation and must agree that
> > it wasn't quite right (mainly due to it not working with inheritance).
> > We have rewritten it now (major change!!!) because we didn't want PHP 5 to
> > be released with a fundamentally flawed mechanism.
> >
>
>Zeev,
>
>Sounds good.
>
>However let me precise some troubles with the new implementation.
>
>1/ Imagine a class with members, but it does not implement __clone because
>the bit for bit copy is OK. Now you derive this class and you need to
>implement __clone. Because the first class does not implement it, you
>can't call parent::__clone (I assume). So this is the responsibility of
>the derived class to clone the base class. This is not only not clean, but
>private members won't be visible.
>
>2/ It still does not help what I already mentioned: when a class
>implements __clone(), all its derived classes have also to implement it to
>transfer their own members.
>
>For sure, in these 2 cases there are work arounds:
>  * case 1: modify the base class to implement __clone,
>  * case 2: implement __clone in the derived classes.
>
>Another idea for case 1 could be to be able to call parent::__clone even
>if the class does not implement it and have the bit for bit copy (but just
>for the base class).

Both are valid points, and they both sound like the same problem from two
different angles.  I think that the best approach would be to first do an
implicit clone (i.e., 'dumb' copy of all of the elements), and then call
__clone(), even if it exists.  It should solve both cases.

Zee

Stephane Drouard

22 years ago

== Quote from Zeev Suraski (zeev@zend.com)'s article

> Both are valid points, and they both sound like the same problem from two
> different angles.  I think that the best approach would be to first do an
> implicit clone (i.e., 'dumb' copy of all of the elements), and then call
> __clone(), even if it exists.  It should solve both cases.

Do you mean that PHP does this bit for bit copy before calling __clone? If yes, this was my original request, so I will be very happy.

Stephane

Andi Gutmans

22 years ago

At 06:49 PM 2/2/2004 +0000, Stephane Drouard wrote:

>== Quote from Zeev Suraski (zeev@zend.com)'s article
> > Both are valid points, and they both sound like the same problem from two
> > different angles.  I think that the best approach would be to first do an
> > implicit clone (i.e., 'dumb' copy of all of the elements), and then call
> > __clone(), even if it exists.  It should solve both cases.
>
>Do you mean that PHP does this bit for bit copy before calling __clone? If
>yes, this was my original request, so I will be very happy.

Yeah, that's what we'll do.

Andi

Stanislav Malyshev

22 years ago

ZS>> Both are valid points, and they both sound like the same problem from
ZS>> two different angles.  I think that the best approach would be to
ZS>> first do an implicit clone (i.e., 'dumb' copy of all of the
ZS>> elements), and then call __clone(), even if it exists.  It should
ZS>> solve both cases.

If we do this, there's no reason for __clone to have parameter - $this
should already have all the data one may need, not?

--
Stanislav Malyshev, Zend Products Engineer
stas@zend.com  http://www.zend.com/ +972-3-6139665 ext.109

Andi Gutmans

22 years ago

At 10:46 AM 2/3/2004 +0200, Stanislav Malyshev wrote:

>ZS>> Both are valid points, and they both sound like the same problem from
>ZS>> two different angles.  I think that the best approach would be to
>ZS>> first do an implicit clone (i.e., 'dumb' copy of all of the
>ZS>> elements), and then call __clone(), even if it exists.  It should
>ZS>> solve both cases.
>
>If we do this, there's no reason for __clone to have parameter - $this
>should already have all the data one may need, not?

You might have a point there.

Andi

Ferdinand Beyer

22 years ago

On 2 Feb 2004 at 14:36, Zeev Suraski wrote:

> Using clone directly is now done using
> $replica = clone $src;
> Clone methods must now be declared as follows:
> function __clone($that)
> {
> }

Finally! I've suggested this change several times before
(engine2@lists.zend.com) but no one was interested (0 feedback).

But now that it will be included I'm very happy!

--
Ferdinand Beyer
<fb@fbeyer.com>

Ferdinand Beyer

22 years ago

On 2 Feb 2004 at 14:36, Zeev Suraski wrote:

> Clone methods must now be declared as follows:
> function __clone($that)
> {
> }

I just had a look at zend_compile.c - it seems as if the variable name
$that was mandantory:

[...]
|| strcmp(CG(active_op_array)->arg_info[0].name, "that")!=0)) {
zend_error(E_COMPILE_ERROR, "The clone method
must be declared as __clone($that)");
[...]

Why shouldn't I be able to name it as I want, e.g. $clone? Don't
understand the reason for that...

--
Ferdinand Beyer
<fb@fbeyer.com>

Steph

22 years ago

> I just had a look at zend_compile.c - it seems as if the variable name
> $that was mandantory:
>
> [...]
>  || strcmp(CG(active_op_array)->arg_info[0].name, "that")!=0)) {
>                   zend_error(E_COMPILE_ERROR, "The clone method
> must be declared as __clone($that)");
> [...]
>
> Why shouldn't I be able to name it as I want, e.g. $clone? Don't
> understand the reason for that...

$that has to be at least as userfriendly as $this - and easier to maintain
than having it be more flexible would.

Andi Gutmans

22 years ago

At 07:32 PM 2/2/2004 +0100, Ferdinand Beyer wrote:

>On 2 Feb 2004 at 14:36, Zeev Suraski wrote:
>
> > Clone methods must now be declared as follows:
> > function __clone($that)
> > {
> > }
>
>I just had a look at zend_compile.c - it seems as if the variable name
>$that was mandantory:
>
>[...]
>  || strcmp(CG(active_op_array)->arg_info[0].name, "that")!=0)) {
>                   zend_error(E_COMPILE_ERROR, "The clone method
>must be declared as __clone($that)");
>[...]
>
>Why shouldn't I be able to name it as I want, e.g. $clone? Don't
>understand the reason for that...

Because we want to keep it standard so that everyone uses the same thing
(like previously).
Anyway, $clone is semantically wrong because it implies that is the cloned
object and not the to-be-cloned object.

Andi

George Schlossnagle

22 years ago

On Feb 2, 2004, at 2:39 PM, Andi Gutmans wrote:

> At 07:32 PM 2/2/2004 +0100, Ferdinand Beyer wrote:
>> On 2 Feb 2004 at 14:36, Zeev Suraski wrote:
>>
>> > Clone methods must now be declared as follows:
>> > function __clone($that)
>> > {
>> > }
>>
>> I just had a look at zend_compile.c - it seems as if the variable name
>> $that was mandantory:
>>
>> [...]
>>  || strcmp(CG(active_op_array)->arg_info[0].name, "that")!=0)) {
>>                   zend_error(E_COMPILE_ERROR, "The clone method
>> must be declared as __clone($that)");
>> [...]
>>
>> Why shouldn't I be able to name it as I want, e.g. $clone? Don't
>> understand the reason for that...
>
> Because we want to keep it standard so that everyone uses the same
> thing (like previously).
> Anyway, $clone is semantically wrong because it implies that is the
> cloned object and not the to-be-cloned object.

If you force the parameter to be name $that, what's the point of
requiring it to be passed at all?  Seems analogous to having all
methods be required to pass $this as their first parameter.

George

Ferdinand Beyer

22 years ago

On 2 Feb 2004 at 21:57, Andi Gutmans wrote:

> Because it's a good way of retrieving the to-be-cloned object (no

behind

> the scenes magic), and in my opinion, it's cleaner because

everyone's clone

> functions will look the same and it'll make it easier to understand

them.

> Why do you care so much if it's called $that or $foobar?
> What is important is that the problems with clone are fixed

(almost). And

> you now have a consistent way of calling the parent clone method

if you

> choose to.

I like the new way very much but I dislike that PHP tells me how to
name a function parameter. __set() and __get(), for instance, do not
force fixed argument names either, do they?

Some people use their own naming convention for parameters and
will be confused when they can't keep it for __clone(). For example
many beginners like to use names in their native language.

I'm sure there will be many people complaining if this stays as it is.

I still don't see the need for a fixed name... it is like forcing every
exception class to have the word 'exception' in their name, just for
consistency....

--
Ferdinand Beyer
<fb@fbeyer.com>

Stephane Drouard

22 years ago

== Quote from Ferdinand Beyer (fb@fbeyer.com)'s article

> I like the new way very much but I dislike that PHP tells me how to
> name a function parameter. __set() and __get(), for instance, do not
> force fixed argument names either, do they?

I agree.

Anyway, do we now need $that to be passed as $this is already initialized with the same values? Except if we want to modify the cloned object, but I'm not sure this is a good idea.

Stephane

Andi Gutmans

22 years ago

At 09:15 PM 2/2/2004 +0100, Ferdinand Beyer wrote:

>I like the new way very much but I dislike that PHP tells me how to
>name a function parameter. __set() and __get(), for instance, do not
>force fixed argument names either, do they?
>
>Some people use their own naming convention for parameters and
>will be confused when they can't keep it for __clone(). For example
>many beginners like to use names in their native language.
>
>I'm sure there will be many people complaining if this stays as it is.
>
>I still don't see the need for a fixed name... it is like forcing every
>exception class to have the word 'exception' in their name, just for
>consistency....

OK, I'll think about it.

Andi

Andi Gutmans

22 years ago

At 02:42 PM 2/2/2004 -0500, George Schlossnagle wrote:

>On Feb 2, 2004, at 2:39 PM, Andi Gutmans wrote:
>
>>At 07:32 PM 2/2/2004 +0100, Ferdinand Beyer wrote:
>>>On 2 Feb 2004 at 14:36, Zeev Suraski wrote:
>>>
>>> > Clone methods must now be declared as follows:
>>> > function __clone($that)
>>> > {
>>> > }
>>>
>>>I just had a look at zend_compile.c - it seems as if the variable name
>>>$that was mandantory:
>>>
>>>[...]
>>>  || strcmp(CG(active_op_array)->arg_info[0].name, "that")!=0)) {
>>>                   zend_error(E_COMPILE_ERROR, "The clone method
>>>must be declared as __clone($that)");
>>>[...]
>>>
>>>Why shouldn't I be able to name it as I want, e.g. $clone? Don't
>>>understand the reason for that...
>>
>>Because we want to keep it standard so that everyone uses the same thing
>>(like previously).
>>Anyway, $clone is semantically wrong because it implies that is the
>>cloned object and not the to-be-cloned object.
>
>If you force the parameter to be name $that, what's the point of requiring
>it to be passed at all?  Seems analogous to having all methods be required
>to pass $this as their first parameter.

Because it's a good way of retrieving the to-be-cloned object (no behind
the scenes magic), and in my opinion, it's cleaner because everyone's clone
functions will look the same and it'll make it easier to understand them.
Why do you care so much if it's called $that or $foobar?
What is important is that the problems with clone are fixed (almost). And
you now have a consistent way of calling the parent clone method if you
choose to.

Andi

George Schlossnagle

22 years ago

On Feb 2, 2004, at 2:57 PM, Andi Gutmans wrote:

> At 02:42 PM 2/2/2004 -0500, George Schlossnagle wrote:
>>
>> If you force the parameter to be name $that, what's the point of
>> requiring it to be passed at all?  Seems analogous to having all
>> methods be required to pass $this as their first parameter.
>
> Because it's a good way of retrieving the to-be-cloned object (no
> behind the scenes magic), and in my opinion, it's cleaner because
> everyone's clone functions will look the same and it'll make it easier
> to understand them.
> Why do you care so much if it's called $that or $foobar?

I wouldn't say that I care 'so much', but since it's on  the cusp of
change I thought I would throw in my two cents.  When you have a
variable with a fixed position and a fixed name, I don't really see the
point in specifying it at all on the command line (i.e. it's like
having to pass 'self' into all your python methods, which seems
annoying).

And if you do have to pass it, I don't see the value in standardizing
the name - you aren't forced to use $key and $value in __get/__set, so
why here?

Like I said, I don't really care, it just smacks of inconsistency
enough to make me send one (or two now, I guess) mails.

George

Adam Maccabee Trachtenberg

22 years ago

On Mon, 2 Feb 2004, Andi Gutmans wrote:

> At 02:42 PM 2/2/2004 -0500, George Schlossnagle wrote:
> >If you force the parameter to be name $that, what's the point of requiring
> >it to be passed at all?  Seems analogous to having all methods be required
> >to pass $this as their first parameter.
>
> Because it's a good way of retrieving the to-be-cloned object (no behind
> the scenes magic), and in my opinion, it's cleaner because everyone's clone
> functions will look the same and it'll make it easier to understand them.
> Why do you care so much if it's called $that or $foobar?
> What is important is that the problems with clone are fixed (almost). And
> you now have a consistent way of calling the parent clone method if you
> choose to.

I don't think George really objects to the name itself as much as he's
wondering why it has to have a fixed name at all.

Forcing an argument to have a specific name really seems inconsistent
with everything else in the language. Following the same logic, we
also should make __get() and __set() have fixed parameter names, but I
don't see anyone arguing for that.

-adam

--
adam@trachtenberg.com
author of o'reilly's php cookbook
avoid the holiday rush, buy your copy today!