Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:51245
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain yahoo.com.au from 98.139.91.82 cause and error)
DomainKey-Status: good
DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws;
  s=s1024; d=yahoo.com.au;
  h=DKIM-Signature:Received:X-Yahoo-SMTP:X-YMail-OSG:X-Yahoo-Newman-Property:Message-ID:Date:From:User-Agent:MIME-Version:To:CC:Subject:References:In-Reply-To:Content-Type:Content-Transfer-Encoding;
  b=iucpGfDvnIKzUuRk4aInzL2FxlLLlGfeRfjfJakQfJx1PIDtIS6oUulFmGDkdXDsnxUBWUAftLxXobnvibkQsBiggcNeI0Klizt6JEvscKkoYXMDTyqDXTWjMFSYyAUbMdQOSmyuQPJbw+lBbsYRVnUmUORLP7vsQZepgU7xkGY=  ;
Message-ID: <4D287BF0.2030105@yahoo.com.au>
Date: Sun, 09 Jan 2011 02:00:00 +1100
User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-GB; rv:1.9.2.13) Gecko/20101207 Thunderbird/3.1.7 ThunderBrowse/3.3.4
MIME-Version: 1.0
To: Stefan Marr <php@stefan-marr.de>
CC: internals@lists.php.net
References: <4D206CA4.6060004@yahoo.com.au> <B7C890E0-75D0-42F7-9A85-AAC57CBB20B8@stefan-marr.de>
In-Reply-To: <B7C890E0-75D0-42F7-9A85-AAC57CBB20B8@stefan-marr.de>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Extensions to traits
From: mail_ben_schmidt@yahoo.com.au (Ben Schmidt)

Hi, Stefan,

I think if the other stuff goes ahead, we can probably scrap what I
originally proposed here. But perhaps something else would be helpful. I
won't comment very specifically on aspects of my original proposal, but
will just raise some new ideas for consideration and further thought.

It all hinges on aliasing: how it works and how it can be avoided when
necessary. Your comment on my example made this most clear.

>>    abstract class ActiveRecord {
>>       protected $new;
>>       protected $id;
>>       protected $other_values;
>>       protected function __construct($id,$values,$new) {
>>          $this->id=$id;
>>          $this->other_values=$values;
>>          $this->new=$new;
>>       }
>>       public function save() {
>>          if ($this->new) {
>>             if (!create_in_the_database()) return false;
>>             if ($this->id===null) $this->id=last_insert_id();
>>          } else {
>>             if (!update_in_the_database()) return false;
>>          }
>>          return true;
>>       }
>>       public static function new() {
>>          return new static(null,static::$default_values,true);
>>       }
>>       public static function get($id) {
>>          return new static($id,get_from_the_database(),false);
>>       }
>>    }
>>    trait LoggingOperations {
>>       public function save() {
>>          if ($this->new) {
>>             log("Creating ".get_called_class());
>>          } else {
>>             log("Updating ".get_called_class()." ID ".$this->id);
>>          }
>>          if (!prev::save()) {
>>             log("Failed");
>>             return false;
>>          }
>>          log("Succeeded");
>>          return true;
>>       }
>>    }
>>    trait EnsuringNoConcurrentChanges {
>>       trait $original_values = array();
>>       protected function setOriginalValues($values) {
>>          trait::$original_values = $values;
>>       }
>>       public static function get($id) {
>>          $record = prev::get($id);
>>          $record->setOriginalValues($record->other_values);
>>          return $record;
>>       }
>>       public function save() {
>>          $current_values=select_from_database();
>>          if ($this->new&&$current_values) return false;
>>          if (!$this->new&&!$current_values) return false;
>>          if ($current_values!=trait::$original_values) return false;
>>          return prev::save();
>>       }
>>    }
>>    trait UsingHashesForIDs {
>>       public function save() {
>>          if ($this->id===null) $this->id=random_hash();
>>          return prev::save();
>>       }
>>    }
>>    class SessionRecord extends ActiveRecord {
>>       protected static $default_values=array(
>>          'user'=>'',
>>          'time'=>''
>>       );
>>       use UsingHashesForIDs;
>>    }
>>    class Client extends ActiveRecord {
>>       protected static $default_values=array(
>>          'user'=>'',
>>          'name'=>'',
>>          'address'=>''
>>       );
>>       use EnsuringNoConcurrentChanges, LoggingOperations {
>>          save = EnsuringNoConcurrentChanges::save,
>>                LoggingOperations::save;
>>       }
>>    }
>
> Ok, that example is, well, not actually helping you.
>
> You can do all the things here without your extensions, I believe.

Before commenting speciically I'd like to point out that whether
something *can* be done shouldn't be the only thing considered. Other
important aspects are how easily it can be done, how tidily it can be
done, how much code duplication it requires, and so on. Sometimes, a
construct by merely providing elegance, without any additional power, is
worthwhile. In fact, you *can* model any program as a Turing machine, so
we actually need very little; but there aren't many popular languages
like that!

In this case, though, I think there are a couple of shortcomings in the
current trait behaviour that mean this can't quite be done reliably.

One thing of note is the use of the trait-scoped property, making the
trait more robust. That was the subject of an earlier email, and one of
the parts of the proposal you viewed most favourably, so I'm happy about
that. I won't dwell on it here.

> Your problem with the save methods is solved by aliases and parent::
> (parent:: only has semantics with respect to inheritance, so it will
> do the right thing.
>
> Your Client will have a save method that calls the aliased versions of
> the saves in the order you want.
>
> And in get() you can also just use parent::, no?

Aliases come a long way, and to be honest, I am still coming to terms
with how powerful they are, particularly in combination with traits'
ability to have abstract methods.

But there is at least one missing link.

You can't reliably use parent:: in a trait method, because that
jeopardises the reusability of the trait. In some composing classes, you
might want it to call a method that isn't from the superclass. In this
minimal example, parent:: happens to work; but that shouldn't be relied
upon when writing a trait, and in the generalised case, where there are
more traits and more composition combinations, it would fall apart.

This means at the very least, you would have to use a method declared in
the trait, and then write forwarding methods. In this case, you have
basically returned to use of the delegate pattern--traits have lost
their benefit. It could look something like this:

use EnsuringNoConcurrentChanges, LoggingOperations {
    LoggingOperations::save insteadof EnsuringNoConcurrentChanges::save;
    EnsuringNoConcurrentChanges::save as saveForLoggingOperations;
}
private function saveForEnsuringNoConcurrentChanges() {
    return parent::save();
}

My original proposal tried to solve this by introducing a prev:: scoping
keyword which worked like parent:: but on a smaller scale, so could take
its place in appropriate methods. I don't deny, though, that having
parent:: and prev:: filling such similar, but possibly confusingly
distinct, functions was not ideal.

Perhaps I was aiming at the wrong target, though. Perhaps what we need
to do to solve this problem is extend the aliasing functionality.

Suppose inside the 'use' block we can have not just exclusion
directives, and aliasing directives for trait methods, but also aliasing
directives for other methods. This may well solve the problem. It
removes the burden from the programmer to write a lot of glue code. I
think we end up with a lot of messy names, though, if we have to use
things like saveForLoggingOperations to ensure traits are linked
together properly. To mitigate this, I suggest we also allow SomeTrait::
to be used after 'as' with the meaning that the aliasing is to an
abstract trait-level access method; this is slightly better. Also, by
making use of the ability to change method visibility, methods in traits
designed to be strung together like this could have trait-level
visibility and be 'publicised' in the 'use' directive, without having to
list a whole lot of clashing methods (as other trait-level access
methods would simply not clash, as they wouldn't be visible). So it
could look more like this:

use EnsuringNoConcurrentChanges, LoggingOperations {
    LoggingOperations::save as public;
    EnsuringNoConcurrentChanges::save as LoggingOperations::prev_save;
    parent::save as EnsuringNoConcurrentChanges::prev_save;
}

This is significantly more elegant, IMHO.

In fact, this is so nice, could I suggest it would be nice to allow
other delegation-like forwarding to be done like this? You could have
'use' without a trait even, just like this:

use {
    $queue->add as addToQueue;
}

Since the properties' object wouldn't be available at compile time, this
extra ability would probably have to be implemented by basically
generating an addToQueue method, and it wouldn't work with arguments
passed by reference. It would basically be a shorthand for

public function addToQueue() {
    return call_user_func(array($queue,'add'),func_get_args());
}

but much more elegant.

I think this achieves everything I was aiming for with my original
proposal, and in a nicer way.

What do you think?

If nothing else, the discussion is definitely worthwhile. I hope we can
get this revised extension, or a later revision, into PHP, so we truly
reap the benefits of the great discussion.

>> Final note regarding grafts
>> ===========================
>
> Grafts are dead. There is no implementation, and I do see traits and
> grafts as an either the one or the other, but not both. We do not need
> to make PHP more complex...

Could that be noted in the RFC, please?

It would be good to note the same in this one, too:

http://wiki.php.net/rfc/nonbreakabletraits

I only found out that it was declined by chance when I happened to see
where it was listed in the 'parent page':

http://wiki.php.net/rfc

If I hadn't decided to have a look at that page for another reason, I
wouldn't have known that a resolution had been reached.

> All in all, I would like to have all the proposals recorded somewhere,
> in a way easily findable when you look up traits RFCs. Should be
> possible to group them in a namespace on the wiki, right?

I will pull together all this new stuff into a new RFC on the wiki,
which can serve as a more useful ongoing reference point at least for
this discussion. Should I put the original proposal in an RFC too, or
should we just let that die and I put the revised one up?

And if I need to do anything to ease the gathering of these RFCs into a
namespace, just let me know.

> Thanks again and sorry for 'rejecting' most of your ideas, but thats
> not final, I am still open for arguments.

No, by all means! The discussion is making the proposal better, and that
won't happen if you don't reject the bad ideas! Only if you rejected
something without good reason would there be any need to apologise. :-)

> By breaking up the topics into subthreads, it hopefully makes it
> easier for the community to comment on the different topics, too.

I think having it online shortly as an RFC might help too.

Thanks again,

Ben.