Advice overridable classes in README.
| Project: | Autoload |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | won't fix |
Jump to:
Really great module!
I think one of the most powerful features on it comes from the fact that it implements an alter hook, that will allow any module to override any class definition from other module. However, when you think of overriding a single class and provide a replacement for it, it somehow clashes with the advice in the README.txt file:
"Do not fall into the easy trap of having one class per file!"
About the performance implications, including more than one class per file may also have the drawback of ending up loading other class definitions that are not used at the end, thus parsing not needed php code. Even if your module always uses two classes together, there may be other modules later reusing your class library differently, and this is what we should promote IMHO.
So, I think you'd better advice 'Do not fall into the easy trap of having all classes in one file', as big includes are more the php way people are used to because of that performance reasons. It is just the wording, but as I see it now, it seems to encourage bundling multiple classes in one file...
Btw, a nice default would be to allow an empty file name and default it to 'MyClassName.class.inc' in the module folder.
Well, I admit I have some Java (one class per file) background though very little PHP OOP experience, so you may weight these factors differently. Just please consider it.
#1
One class per file in PHP is actually a very bad idea, IMO, unless you're using an opcode cache. The disk IO of loading a file is more expensive than parsing the code in many cases, especially if the file is relatively small. Most of those in the PHP OOP world arguing for one-class-per-file do so because (1) doing that and very long class names that match the path makes the autoload routine very simple (but frankly Drupal's is way better :-) ) and (2) they're using an opcode cache and can't comprehend someone not doing so for any "serious" application.
Since the vast majority of Drupal sites do not use an opcode cache, that's the use case we need to keep in mind.
You do, however, raise one interesting point. If you want to change the file in which a given class lives in order to swap it out, you have to do the same to every class in the module. I'm not actually sure that's a major problem in practice, as if you want your classes to be swappable you should be using Handlers instead anyway, but it could be an issue. It depends how people end up using this module.
I'm going to leave this open and see if anyone gives feedback on that point.
#2
After a year, I don't think this is really an issue. :-)