Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:56270
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.160.170 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <4EBC9710.5010807@gmail.com>
References: <CAAyV7nEi+1XzYryoE9tg48jZMMPe1psz=VWDkGSzEgAwBKs9pw@mail.gmail.com>
 <CAFp73XuQGG9mEZUZ2OK1yLkBiFDcPwOxytQ80D=xZUafEv20tw@mail.gmail.com> <4EBC9710.5010807@gmail.com>
Date: Fri, 11 Nov 2011 01:49:49 -0200
Message-ID: <CAFp73Xs2=7BfUjKfxpu0xFZwQd+YhiBjW=O-FB3M+z1yQrXv+g@mail.gmail.com>
To: David Muir <davidkmuir@gmail.com>
Cc: Anthony Ferrara <ircmaxell@gmail.com>, PHP Internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Re: SPLClassLoader RFC Thoughts And Improvements
From: guilhermeblanco@gmail.com ("guilhermeblanco@gmail.com")

Hi David,

On Fri, Nov 11, 2011 at 1:31 AM, David Muir <davidkmuir@gmail.com> wrote:
> On 11/11/2011 01:31 PM, guilhermeblanco@gmail.com wrote:
>> Hi Anthony,
>>
>> Thanks immensely for your input.
>> Without such action, it's extremely hard to improve the RFC. =3D)
>> Awesome action from your side.
>> I'll place my comments inline.
>>
>> On Thu, Nov 10, 2011 at 1:26 PM, Anthony Ferrara <ircmaxell@gmail.com> w=
rote:
>>
>
> ...snip...
>
>>> =C2=A0b) The code that includes the file should be changed to require_o=
nce
>>> to eliminate including the same file twice when different classes map
>>> to the same file. =C2=A0For example, this should not fail (assuming bot=
h
>>> classes are not in the same file):
>>>
>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0new \Foo\Bar\Baz;
>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0new \Foo\Bar_Baz;
>>>
>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0Both map to the same file, but are distinct =
classes in PHP.
>>> By changing it to require_once, it'll realize it already included the
>>> file and then skip over that and let another class have its shot.
>>>
>> Actually they do not map the same file. Here is the path of each one:
>> new \Foo\Bar\Baz; =3D> [path]/Foo/Bar/Baz.php
>> new \Foo\Bar_Baz; =3D> [path]/Foo/Bar_Baz.php
>>
>> You may now be in doubt if PEAR1 style is supported. Yes, it is. The
>> point is that code detects for the presence of namespace separator and
>> if it is found, it considers the namespace separator as directory
>> separator; if not, it considers the "_" char as directory separator.
>> We decided that because PEAR1/Zend1/Doctrine1/Symfony1 (and many
>> others) would not be supported if we didn't do that way. It's the
>> solution for BC. Since most of the mentioned ones are already in
>> another major version, consuming PHP 5.3 namespaces, then the default
>> behavior would work nifty. =3D)
>
> This is news to me, and even the RFC states:
> Each =E2=80=9C_=E2=80=9D character in the CLASS NAME is converted to a
> DIRECTORY_SEPARATOR. The =E2=80=9C_=E2=80=9D character has no special mea=
ning in the
> namespace.
>
> I believe you're confusing it with the namespace portion:
> new \Foo_Bar\Baz; =3D> [path] /Foo_Bar/Baz.php
> in which case the _ is retained.
> Otherwise:
> new \Foo\Bar_Baz; =3D> [path] /Foo/Bar/Baz.php
> new \Foo_Bar\Baz_Waz =3D> [path] /Foo_Bar/Baz/Waz.php
>
> If what you are saying is true, then the RFC is incorrect, and should be
> updated to included the correct rules.

You're completely right. Sorry for my mistake.
I overlooked it tonight... it might be my lack of some sleep these days. =
=3D\

>
>>
>> Regarding the require_once change, it's not really needed. The reason
>> is because class loading would never be triggered again if the
>> class/trait/interface is already loaded. The require_once is extremely
>> slow than require due to this check.
>> But I really don't have strong feelings to change that is many people
>> find it necessary.
>
> That was true 6 years ago. Benchmarks from 4 years ago, it was the other
> way around. Has it flipped back again?
>

I remember that 2 years ago we did benchmarks around
require/require_once and the latter was slower by a factor of 30%.

> ...snip...
>
>>> 4. The RFC should avoid implementing any pattern or style that may
>>> make future feature addition difficult or pose risks towards such. =C2=
=A0An
>>> example would be implementing an interface for the autoloader which
>>> defines something like load($class). =C2=A0The problem there is that if
>>> function autoloading is added, the interface won't be able to support
>>> it. =C2=A0So it's stuck in a hard place between changing an implemented
>>> interface (which will bork code significantly on update) and adding a
>>> new interface (which would be the lesser of evils, but would just add
>>> to the deprecated cruft).
>> OO theory defines that whenever you want to have a contract to be
>> followed by any implementation, an interface is required.
>> The contract is something useful that enforces that way you receive
>> would have at least what it was expected/used internally.
>> So, if we agree in the future that the chosen implementation would not
>> contain the methods ->register/->unregister, and spl_autoload_register
>> would accept SplAutoload instances, then the contract is more than
>> required.
>> Regarding the function autoloading, as I discussed previously, it is
>> invalid in this paradigm.
>> I don't see how the contract would change in the future.
>
> But the only contract that is needed by spl_autoload_register is the
> load method.

Agree.

>
> The register and unregister methods are not needed by any other entity
> that I am aware of, and are only there as convenience methods for the
> user, and therefore don't belong in the interface for a generic
> autoloader. A second interface could be added for self-registering
> autoloaders, but that begs the question, why should an autoloader know
> how to register itself with a particular loading stack (sure the spl
> autoloading stack is the only one we have, but still...)? There's no
> discussion in the RFC showing why those methods should be part of an
> autoloader interface.
> The same applies to the setMode() and add() methods. What contract are
> they fulfilling?

The add was a useful inclusion to keep consistency of that every
SplAutoload should be able to deal with > 1 namespace. It's not
mandatory and if everyone agrees that it should be taken out, I'll
remove.

The setMode method becomes invalid if we drop the MODE_NORMAL support.
Of course there will have the MODE_DEBUG yet to address, but it can be
removed from SplAutoload contract.

>
> Cheers,
> David
>



--=20
Guilherme Blanco
Mobile: +55 (11) 8118-4422
MSN: guilhermeblanco@hotmail.com
S=C3=A3o Paulo - SP/Brazil