Renaming HTTPResponse and Query classes?
Sam Minnee
in certain PHP extensions.
See http://open.silverstripe.com/ticket/2827 for more info.
1) Should we rename them to SSHTTPResponse and SSQuery? I don't think
they get used outside of the core too much... we would have to provide
users with upgrade instructions telling them to search their own code
for "HTTPResponse"
2) Are there other classes which are particularly at risk of name
collisions?
3) Is there a better way that we can manage this?
Ingo Schommer
for PHP 5.3 to become a minimum requirement and use namespacing,
which is realistically still two years away (if providers even bother
upgrading
their hosting before PHP 6 is released).
We've already started the pseudo-namespacing with "SSDateTime",
but now we have ugly names like "USSSDateTime" - it's just not very
readable.
I'd much prefer the "SS_" prefix.
Its actually good timing to make this change, with a major release (2.3)
being prepared. The question is, are we chasing this up time and again
and keep breaking our API bit by bit? Is there a definitive list of
reserved names for PHP 5.3 and PHP 6, and any popular extensions?
We've got heaps of common names which could collide.
Consequently we should namespace *all*core classes - but
this will mean rewriting basically every legacy application under the
sun,
plus a lot of documentation. We could mitigate this effort
with some readymade regex-rules, but its still gonna be a royal pain.
So question is, do we have a workable system with ad-hoc fixing until we
can require 5.3?
-------
Ingo Schommer | Senior Developer
SilverStripe
http://silverstripe.com
Phone: +64 4 978 7330 ext 42
Skype: chillu23
Andrew Short
underscore - otherwise it just ends up looking ugly.
An interesting approach that I've come across before that would solve
namespacing issues, while not breaking backwards compatibility is
similar to that used by the Kohana (kohanaphp.com) framework. In their
autoload function, if the requested class cannot be located, then it
searches for a namespaced class (e.g. if Controller cant be located,
it searches for SS_Controller). If it locates this class, then it
dynamically creates a class that extends this class using an eval()
statement.
This approach has a number of advantages that I can immediately see:
1) It solves namespacing issues - within the actual framework, we
should use SS_Controller extends SS_RequestHandlingData - as this will
stop conflicts from happening within the core. However, site creators
will still be able to use MyController extends Controller - as this
class will get dynamically created.
2) It would allow easier code injection - currently, we have the
Object::create() and related methods - but you cant overload something
like CoreClass::function() with your own class. If this change were to
be made, then all you would have to do is create a class called
Controller, and it would override SS_Controller globally - and no more
messing with Object::create() - you can just use a plain new operator.
3) Its fairly easy to implement. Itll require some changes to the
ClassInfo class, and the autoload function, but its not a majorly deep
change. It also means that it can be implemented in stages (you dont
have to rename every class at once!)
Ingo Schommer
On 23/09/2008, at 12:40 PM, Andrew Short wrote:
>
> An interesting approach that I've come across before that would solve
> namespacing issues, while not breaking backwards compatibility is
> similar to that used by the Kohana (kohanaphp.com) framework.
The autoloading happens here:
http://dev.kohanaphp.com/browser/trunk/system/classes/kohana.php#L841
> In their
> autoload function, if the requested class cannot be located, then it
> searches for a namespaced class (e.g. if Controller cant be located,
> it searches for SS_Controller). If it locates this class, then it
> dynamically creates a class that extends this class using an eval()
> statement.
from some other PHP package, no namespaced class will be
created or used by application code, right? Our autoloader
would have no way of telling which of the two classes the
application developer intended to use. So we're back to square one?
Using eval() in such a broad context seems like a pretty bad hack,
plus a performance hit (28x slower than normal code).
See http://blog.joshuaeichorn.com/archives/2005/08/01/using-eval-in-php/
>
>
> 2) It would allow easier code injection - currently, we have the
> Object::create() and related methods - but you cant overload something
> like CoreClass::function() with your own class. If this change were to
> be made, then all you would have to do is create a class called
> Controller, and it would override SS_Controller globally - and no more
> messing with Object::create() - you can just use a plain new operator.
Rails devs tend to get into trouble by this form of "monkeypatching
with mixins" -
if customization is needed that can't be solved with
DataObjectDecorator,
ss devs should rather request hooks or config-options IMHO.
Object::create() was mainly used for overloading of the Member object
a while ago, which can now be achieved by decorating.
Do you have any use cases for replacing core classes?
>
>
> 3) Its fairly easy to implement. Itll require some changes to the
> ClassInfo class, and the autoload function, but its not a majorly deep
> change. It also means that it can be implemented in stages (you dont
> have to rename every class at once!)
Thats a big benefit of course.
Hm, definetly worth a look but I'm not convinced - Sam? :)
Andrew Short
> But that means that if you already have a class HTTPResponse
> from some other PHP package, no namespaced class will be
> created or used by application code, right? Our autoloader
> would have no way of telling which of the two classes the
> application developer intended to use. So we're back to square one?
approach, we can either have the advantage of less core conflicts OR
easier code injection. (With both maintaining backwards
compatibility).
Within the core code you would strive to use the namespaced classes
(SS_Controller rather the Controller) - and that would eliminate the
core conflicts, while still allowing for backwards compatibility.
Alternatively, you could use Controller in the core. This has the
problem of still encouraging conflicts, but it allows for easier code
injection - which ill explore later.
The one solution I could think of was to use double namespacing in the
core (e.g. SS_Controller_Core). Even though this would only be used in
one place, it could still be cumbersome or unappealing to some. Then
we could extend from SS_Controller in the core - SS_Controller is
dynamically derived from SS_Controller_Core, as is a plain Controller
- but this would still allow for code overloading SS_Controller to
replace core classes, or just overloading Controller to overload the
controller in the mysite context.
The problem with all of these approaches is that it ends up creating
2-3 times the amount of classes normally needed. Id honestly like to
avoid this at all costs - if someone has an alternative suggestion
(that would also maintain backwards compatibility) I'm all for it ;)
> Using eval() in such a broad context seems like a pretty bad hack,
> plus a performance hit (28x slower than normal code).
> See http://blog.joshuaeichorn.com/archives/2005/08/01/using-eval-in-php/
to create a hundred thousand objects using eval(). The speed lost can
easily be regained by simply caching a few things in ManifestBuilder
or some Lazy Loading etc.
> Rails devs tend to get into trouble by this form of "monkeypatching
> with mixins" -
> if customization is needed that can't be solved with
> DataObjectDecorator,
> ss devs should rather request hooks or config-options IMHO.
> Object::create() was mainly used for overloading of the Member object
> a while ago, which can now be achieved by decorating.
> Do you have any use cases for replacing core classes?
config parameters - as theres lots of complicated static calls etc. Or
say I wanted to completely overload the current Security
implementation and make it call out to a remote system for all user
information.
> Thats a big benefit of course.
honestly, as I think about it more I'm beginning to like it less and
less.
Maybe just doing an audit of classes that are very likely to conflict
and then renaming them would be a better approach as it definitely
adheres to the KISS approach, and has less potential for funky bugs.
> Hm, definetly worth a look but I'm not convinced - Sam? :)
Ingo Schommer
On 25.09.2008, at 12:21, Andrew Short wrote:
>
> On Sep 23, 10:01 pm, Ingo Schommer <i...@silverstripe.com> wrote:
>>
>> Using eval() in such a broad context seems like a pretty bad hack,
>> plus a performance hit (28x slower than normal code).
>> See http://blog.joshuaeichorn.com/archives/2005/08/01/using-eval-in-php/
>
> I don't think its that bad - it took just over a second for my machine
> to create a hundred thousand objects using eval(). The speed lost can
> easily be regained by simply caching a few things in ManifestBuilder
> or some Lazy Loading etc.
I don't really see a second of processing time as "fast" ;)
>>
>> Do you have any use cases for replacing core classes?
>
> Nested URLs without hacking the core. This cant be fixed with simple
> config parameters - as theres lots of complicated static calls etc. Or
> say I wanted to completely overload the current Security
> implementation and make it call out to a remote system for all user
> information.
This example is beyond what should be customizeable by subclassing
and monkeypatching, because it affects so many areas - you'd even need
to overload CSS design and Javascript behaviour somehow.
Its core stuff that should be developed in core.
If we see a pattern where a specific featureset needs to be customized
frequently (in a non-core-fashion), we can talk about providing hooks
or easier defined class extension points. One example of this might
be replacing the rich-text-editor, something thats currently baked
into core
with TinyMCE.
I think as long as we have a striving community, a healthy release
process,
and well-documented workflow on how to get in patches,
the need for monkeypatching is greatly reduced.
Another answer here might be a switch to git version control,
which makes branching off core a lot easier.
>
> Maybe just doing an audit of classes that are very likely to conflict
> and then renaming them would be a better approach as it definitely
> adheres to the KISS approach, and has less potential for funky bugs.
Good Idea!
>
>
>> Hm, definetly worth a look but I'm not convinced - Sam? :)
>
> Good to be slightly skeptical ;)
Lets not forget that PHP doesn't really allow for "mixins" or in-
memory class replacement,
so we should evaluate really thoroughly if we need another
"language feature simulation" in SilverStripe - they tend to
make things slow and complicated...
Some opinions about core patching in rails:
http://chadfowler.com/2006/1/26/the-virtues-of-monkey-patching
http://avdi.org/devblog/2008/02/23/why-monkeypatching-is-destroying-ruby/
http://avdi.org/devblog/2008/02/25/followup-to-monkeypatching-is-destroying-ruby/
http://www.codinghorror.com/blog/archives/001151.html
http://blog.nicksieger.com/articles/2008/03/14/monkey-patching-is-part-of-the-diy-culture
http://en.wikipedia.org/wiki/Monkey_patch
Michael Gall
Cheers,
Michael
--
Checkout my new website: http://myachinghead.net
http://wakeless.net