Standardize proxy class naming #125

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Member

schmittjoh commented Sep 18, 2011

No description provided.

Member

stof commented Sep 18, 2011

There is an issue here: as you are placing the proxy in the namespace defined by the user, the autoloading will trigger the autoloader of the user when unserializing a proxy. This is not an issue when using a silently failing autoloader (the Sf2 one for instance) but it is one when using an autoloader than does not fail silently (the Doctrine one for instance).

A better idea could be to prepend a namespace. Entities\User would create the proxy Proxies\Entities\User which does not match the autoloader of the Entities namespace.

Member

stof commented Sep 18, 2011

This is not about the proxy autoloader but about the other autoloaders. If a namespace is registered for a loudly-failing autoloader (like the Doctrine one), it must be able to autoload all classes of this namespace, which will not be the case in your proposal.

Member

stof commented Sep 18, 2011

But this require the user to register this autoloader by prepending it otherwise he will get errors when using the Doctrine autoloader. And this also mean that he will get some issues if he registers another prepended autoloader after this one.

Owner

beberlei commented Sep 18, 2011

Hm, i think prepending a namespace is necessary to avoid confusing with autoloader sorting and such, other than that i think the idea is great. One configuration option less => happier users.

Member

stof commented Sep 18, 2011

Well, if you prepend the namespace, you can keep the ability to configure it (thus avoiding weird statements in all code integrating Doctrine to support the BC break). But a good idea would be to have a default value for the proxy namespace to make users happier

@schmittjoh schmittjoh closed this Sep 23, 2011

beberlei added a commit that referenced this pull request Jan 23, 2013

Merge pull request #125 from Ocramius/patch-4
Update en/reference/tools.rst
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment