The main reason why we don't like eval() is if it is used to execute uncontrolled php code from the database or user input, as it happens a lot in other parts of Drupal.
The pieces of php that are eval'd with stapler are quite simple, with the only unknown part being class names built from file name fragments. (if you know a better way to sanitize class names, let me know!)
Eval'd code is not optimized, either by the PHP interpreter itself, nor by the OPcode caches. Il will totally kill performances to use such mecanism in a global scale.
Comments
Comment #1
pounardPlus you are missing either the code build time (string operations, string are immutable objects and each manipulation will instanciate a lot! And will make the interpreteur allocating and deallocating memory every time). If you fetch this code from database or files, it's even worst because you are making heavy I/O's.
Comment #2
donquixote commentedOh great, someone found this module :)
(i do not use it myself btw)
This is probably a valid point against eval() in general.
The question is, how bad is the impact for this specific module?
The code that is eval'd is actually very very short, so I don't think the direct execution of this code would cause any significant problem.
On the other hand, the module will generate class hierarchies that are "glued together" with eval'd "inherit" statements.
If one class in a chain of inherited classes is eval'd, does that have an impact on the performance of methods being called?
I have no idea. But you are welcome to do some benchmarking :)
Did you actually have a look what the module does?
Comment #3
pounardNo, actually, I only read the description! I'll look at it then get back to see. But honestly, I think that generating classes on the fly, outside of a greater pattern (such as code dynamic objects generation from a model, for example) is quite silly because it's obfuscated and hard to maintain. But if you have a greater design arround it, why not. But still, this module as-is with no big idea behind it freaks me out a bit :)
Comment #4
pounardAnother note, C++ templates are part of the language, and it's really the worst thing a compiler has ever had to compile, ever. If you code C++ using only templates, it's freaking long to compile. I saw a a tic tac toe AI coded only with C++ templates, but not only the code was really ugly and stupidely long, but so was the compile time (almost 3 or 4 hours, on a decent box, only for this).
You probably meant more something like Java's or C#'s generics (which actually is a tiny part of what really are C++ templates). Which is strong-typed checks at compile time but type inference at runtime (something like this), this is something that does not exists in PHP and that will be really hard to implement, even worst if you attempt to create this at runtime.
What you are doing here, is doing exactly what does Java at compile time, it's creating new classes. But java handles it at compile time and the code runs fast! C# uses type inference to make shorcut while resolving class hierarchy therefore make the compilater faster than if you don't use it (especially while using interfaces for type inference) but it also improves IDE for automatic completion and make the code more readable. In your case, it make the code harder to maintain, and it make the code slower.
Comment #5
donquixote commentedDuh. I just found that the subdirectory that is necessary for the examples had not been added to cvs. The module was sitting there all the time, with an incomplete example. This is quite embarassing.
6.x-1.0-alpha5 should fix that.
Comment #6
donquixote commented> C++ and templates
The idea is that you let the compiler do some of the work that would otherwise happen at runtime. This can be a good tradeoff. But it only pays off if you do a lot of micro operations.
----------
I think my module description is not the best..
What I meant with my mention of C++ is the mixin template: You define a class template that gets its parent class from a template argument. This way, you can reuse this class with more than one base class.
Now, stapler does not do that. Every class definition can still only be used once. But now other modules can declare additional parent classes or "layers".
The point is: Usually if you define a class in PHP, you know what the parent is, and the grandparent, etc.
With stapler this not the case: Other modules can "rewrite history", and squeeze new intermediate generations into your chain of parents.
If you like this or not, your choice.
I have never seen a solid use case, and would rather not use this beast.
Comment #7
pounardI'm sorry I was a bit aggressive, I maybe shouldn't. It's an original solution I must admit.
But, I'm quite sure that for every use case you'll find there already are design patterns, such as Decorator, Chain of responsability, Adapter, Proxy, Factory method, etc..