Ridiculously RADiculous

EDIT:An user with the name gwoo corrected an incorrect assertion I made in the comments section:

Your last example shows how an active record can be created and has very little to do with your assertion that it is to “use late static binding inside of closures…” The actual code to use late static binding in an anonymous function is…$class = __CLASS__; $lambda = function() use ($class) { return $class::invokeMethod(); }

—–
I have seen my fair share of absolutely atrocious PHP code, having being called way too many times on projects at a fairly late state only to discover I should have asked for a bonus (in the form of a personal masseur or something similar) just for the headaches.

I’m presently doing some research for a series of paper I plan to publish on bad design decisions plaguing the PHP code base. To this end, I started studying the Lithium 3 framework, which deemed it appropriate to crown itself as the most RAD framework for PHP 5.3+, hence the name of this blog post. I learned about the existence of this framework whilst attending a talk Joël Perras and Nate Abele gave at the Tek-X conference last week, which ended up getting a mitigated reception because of their we’re right, you’re wrong attitude.

Well, at the end of the presentation, I knew something was wrong. I knew I would be surprised if I ever opened the hood to see what kind of black magic happens in there. Boy, was I right !

I won’t enter in the detail of my fundamental disagreements with the design of the framework (for now, this’ll come later on the form of a detailed analysis) but I simply wish to share what I think is one of the funniest piece of code publicly available on the interwabs which actually is distributed shamelessly, to say the least.

public static function invokeMethod($method, $params = array()) {
    switch (count($params)) {
        case 0:
            return static::$method();
        case 1:
            return static::$method($params[0]);
        case 2:
            return static::$method($params[0], $params[1]);
        case 3:
            return static::$method($params[0], $params[1], $params[2]);
        case 4:
            return static::$method($params[0], $params[1], $params[2], $params[3]);
        case 5:
            return static::$method($params[0], $params[1], $params[2], $params[3], $params[4]);
        default:
            return call_user_func_array(array(get_called_class(), $method), $params);
    }
}

That code resides in the lithiumcoreStaticObject. You can browse it online here.

It was love at first sight. Oh dear ! This is so great. Pure genius. Of course, I’m being a bit cynical here. Interestingly enough, Tek-X’s keynote speech, given Josh Holmes, had the title The Lost Art of Simplicity (if I may, I want to mention that it was one of the best talks I have attended in a long time). Well, this code is exactly the opposite of simple. Is the invokeMethod() method called so often that it necessitates such a micro-micro-optimization ? Probably not. And, well, isn’t it a bit like solving a problem that would not have needed to be solved, had sane solutions been applied, i.e. stop using static methods all the time ?

Yes.

From what I could see (and maybe I am wrong), the Li3 guys use this method inside closures to do some black magic.

/* Why is this method named so ambiguously anyway ? */
$self::_instance();

$badAssLambdaFilter = function ($whatever) use ($self) {
    return 'OH HAI '. $self::invokeMethod('getEndOfLine');
}

But wait. Wouldn’t something along the lines of:


class Test {
    public static function aMethod() { echo 'OH HAI !'; }
}

$lambda = function () { Test::aMethod(); };

do the trick ? Ohh wait, no ! Because of Li3’s obsession for anything static (a strange obsession indeed), black magic needs to happen; the static keyword happens to not work inside of a closure, hence, they had to hack their way around this limitation.

Here’s yet another piece of code to entertain you; it’ll show you exactly how they do that:

protected static function &_instance() {
    $class = get_called_class();

    if (!isset(static::$_instances[$class])) {
        static::$_instances[$class] = new $class();
    }

    return static::$_instances[$class];
}

So, in order to use late static binding inside of closures, there’s actually a method that creates “fake” instances which are then passed to the closure as a parameter. This baffles me. Li3 is full of such “anomalies” and cool stuff that does nothing more than being cool. This is broken beyond repairable. I often have reached a point where it seemed my code was broken, not because I knew exactly why it was broken, but because of all the weird hacks I had to use to actually make the code work. In the end, I always end up finding the missing link. In this case, it seems that it has never crossed the minds of the Li3 masterminds that something was evidently wrong.

But in the end, I’m still glad I save a few femtoseconds each time I call invokeMethod(). Hurray !

Advertisements

9 Comments on “Ridiculously RADiculous”

  1. TheLoneCabbage says:

    the reasoning for this in their own words:

    http://rad-dev.org/lithium/wiki/about/FAQ

    In a nutshell: It’s easier to test statics because the way they use them does not depend on system state, only passed parameters.

  2. […] This post was mentioned on Twitter by Jonathan H. Wage, Nicolas A. B.-N.. Nicolas A. B.-N. said: New blog post, this time about #li3 ; http://bit.ly/cJkqzZ. #php […]

  3. Nate Abele says:

    First, I will say that trying to be so insulting really doesn’t help your position, it only makes you seem insecure. Second, the explanations you’re looking for are actually quite simple and have nothing to do with statics. I’ll be sure to speak slowly and use small words when I add *that* to the FAQ. 😉 Kidding, kidding…

    • Nicolas A. Bérard-Nault says:

      You know, it’s just my style, I’m a polemist I guess. Maybe this demonstrates some sort of Freudian insecurity that could be cured by nanobots ? Tell me, oh lighthouse in a sea of suckness that you are my dear Dr. Abele.

      The problem is, your code seems to exacerbate this primal insecurity to a point where I actually suffer epilepsy crises and end up writing blog posts whilst convulsing helplessly on the ground.

      That’s how bad Li3 is.

  4. Nicolas A. Bérard-Nault says:

    I see that you absolutely love your design Nate, but people can disagree with you and it might not only be because they do not understand your “genius” or because they are insecure.

    I didn’t say the code was not clever. I mean, it’s pretty damn clever to invent nanobots that cure all diseases. What is not so clever is to insert nanobots into the bloodstream of a patient if you have to give the guy a drug so that his body does not reject the bots and a drug that counters the side effect of the other drug and yet another drug to counter the side effet of all the other drugs, which ends up costing thousands of dollars and may end up killing the patient.

    Granted, your fucking nanobots are awesome, but I’ll stick with curing my bad cough with a nice cup of warm herbal tea untill you come up with a better way, i.e. one that does not mandate you to produce hackish code.

  5. gwoo says:

    “This is broken beyond repairable.” Actually, this is just code so changes are not too hard to make. For your first example, you could easily use the default case and remove the switch statement. Your second example is actually taken out of context, but it does show how a creative mind can solve interesting problems. In that case, the problem is the hard coded dependency as your 3rd example suggests. Your last example shows how an active record can be created and has very little to do with your assertion that it is to “use late static binding inside of closures…” The actual code to use late static binding in an anonymous function is…$class = __CLASS__; $lambda = function() use ($class) { return $class::invokeMethod(); }

    Hopefully, this helps you understand some of the “clever” approaches inside of Lithium. I agree both nanobots and Lithium are awesome. I would suggest toning down the bombastic language to focus on the merits of your conclusions. You are more than welcome to disagree, in fact it helps spur lively debate and lead to better outcomes. Please keep looking at the code with a skeptical eye and pointing out more “anomalies”.

    • Nicolas A. Bérard-Nault says:

      I agree with you that this part of my post was, at best misguided, but you have to agree with me it was a good guess as I was not too far from the “real answer”, which I still find completly ludicrous.

      I think I was actually referring to this comment, left in the source code by a gentle soul:

      While the `Model` public API does not require instantiation thanks to late static binding introduced in PHP 5.3, LSB does not apply to class attributes. In order to prevent you from needing to redeclare every single `Model` class attribute in subclasses, instances of the models are stored and used internally.

    • Nicolas A. Bérard-Nault says:

      Furthermore, if I may, creative mind or not, a red, blinking alarm should sound in your head when you have to go through such complex circonvolutions simply to do a very simple task. No matter how much the Active Record pattern sucks (and it fucking does), implementing it this way with so little benefit and so many problems to solve reveals is just insane !

      Solving a simple problem by making it complex and then presenting the solution as clever is just misrepresenting the facts. In the end, a simple solution, if it is sufficient, is far superior to any complex, clever solution.

  6. steve rodrigue says:

    Wow, just wow.

    I thought i have seen bad quality hackish untestable code in my life… this is probably is worse than lots of bad code base i have worked in.

    Your paper on bad code and architecture interests me thought… keep me updated on that.

    Oh and guys. NIC have bad ass hard skills, but this example doest show them. He just shows the evidence: a pile of shit with chocolate on it is stiml a pile of shit! Now face it!

    Steve


Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s