• Eric Hodel (unregistered)

    Variable variables make your code smell.

    When programming, you should be able to express yourself in the cleanest way your language provides. Variable variables obscure your code making it harder for people to read, including yourself.

    This gives you too much code, and not enough expression. Too much code gives you code smell.

  • C0b0l (unregistered)

    ALTER is obsolete? How come no one told me?!?!

  • Ron (unregistered)

    I'll call your code smell and raise you a code noisome

  • Ian Bicking (unregistered)

    A certain level of metaprogramming (of which variable-variables are an instance) can be justified. They have to be pretty important or pretty idiomatic to be worth it. A case might be a map function, where you are aware of the idiom, and it will remain fairly localized (particularly because the map implementation is fixed and specified).

    I think this code would be much better if it did something like: $admin = "admin_template_" . $admin; thus constraining admin functions to a certain set of names. In this way you could find the set of functions meant to be used like this, and hopefully if you find such a function you'll be able to track back to this function as well.

    To totally dismiss higher order functions, metaprogramming, and other similar programming constructs would be to dismiss table-driven programming (which a lot of people like a lot), and most systems that support plugins or other loosely coupled constructs (loosely coupled in the sense that they are developed and distributed separately, even though they're might be strong dependencies between them).

  • Rob Menke (unregistered)

    Just to add my two cents to Ian's comment, this really is a simplified form of object-oriented programming but without the language support. If you view each row not as a tuple but as an object, and the admin_function field as a polymorphic method, the parallel is clear.

    Some languages actually use strings and lookup tables to implement object-oriented programming. Perl does, and so does Objective-C. Java allows you to do method dispatch on a string via its reflection package. C++ is the worst as all this information is lost in compilation, which is sad because occasionally it comes in handy.

    So to dismiss this is to dismiss OOP, and everyone loves OOP, right?

  • tic (unregistered)

    I completely agree with Ian on that one; you guys need to widen your horizons a bit and look at other programming paradigms, such as the ones used by Haskell and to some extent Python.

  • tic (unregistered)

    ... that'd be Rob.

  • Gavin Brown (unregistered)

    "Variable variables" (at least, variable functions) are also present in Perl. Consider:

    #!/usr/bin/perl

    my $foo = 'bar';

    sub bar { 'foo' };

    print 'the value of &{$foo}() is '.&$foo()."\n";

    This produces:

    $ perl varvar.pl
    the value of &{$foo}() is foo

    If you use the strict pragma this syntax raises a parse error, but a lot of novices don't use strict. Perl has very nice pragmas for stricture and variable tainting which make using this sort of thing a lot safer, thankfully!

  • esquilax (unregistered)

    metaprogramming is one thing, but pulling function names from the database and executing them is definitely another.

  • Jeff Atwood (unregistered)


    All this highfalutin' theory makes my brain itch.

    All I know is, I would have NO idea what that code did if it wasn't explained below. To say that it is nonintuitive is an epic understatement.

    And whatever the theory, that can't be good.

  • Bart (unregistered)

    Dno, but I use variable-variables very frequently in PHP. This is just an equivalent of (function)pointers in C/C++.
    Through the reflection API in Java you can also do the same as what this guy is doing. Ok, it's a great way to shoot yourself in the foot, but in some cases it is extremely handy.

    I wrote a site-framework and a very easily customizable BB-alike code parser in PHP using this. Why? They're both plugin-based, and I used this to avoid having 100 classes containing each one stupid function, which is far from optimal, and the maintainability is also totally gone. Now I get one module (a class) with multiple handlers in it, which the constructor of this module returns to the "core".

    Once you use some kind of dynamic plugin-based system, it's very hard to do without. It's better to have one rather complicated but well written, well documented and concentrated part than one big mess of an on first sight very good design, but in practice unmaintainable amount of files and classes. Imagine my bb-code parser to use a module for each freakin' tag it can handle.. Now that would be a big mess. Now it's just a class which groups all related tagshandlers that's loaded instead of 30 or 40 classes for each possible tag.

    In some situations it's very hard not to use these kind of practices. You only have to understand em very well before you use em. Guess pointers are also something evil according to those variable-variable haters? They're bloody handy, but you should know what you're doing.

    This doesn't make this WTF any less, putting function names in a database seems to me as an extremely stupid idea...

  • Peter (unregistered)

    >metaprogramming is one thing, but pulling function names from the database and executing them is definitely another.

    I've done that before. A table held the reports/tasks to run during the day, and one of the columns held the name of the DCOM/ActiveX object to run. All the report objects had the same properties and methods, therefore the scheduler did not need to be modified. All you had to do was install the new doohickey and update the table.

    Dim DooHickey as Object
    ...lots omitted...
    Set DooHickey = CreateObject(rs("object_name"),rs("run_at_server_name"))
    DooHickey.RedStapler = rs("parameter")
    DooHickey.TimeToMakeTheDonuts = rs("report_last_run_at_time")
    DooHickey.McRun
    ... ommitted is stuff for keeping track of who is running what and where and glue to handle lost sheep/processes.

    On NT, CreateObject had optional 2nd param for what server do you want your dcom object to run on. The vb-fragment above ran as an NT service. Users could make jobs run NOW_DAMMIT! or let them run at their regularily scheduled time.

    Oh, if you ever have anything to do with writing plug ins for MMC (for the sake of your hair: DON'T), you will be writing stuff where you call procedures whose names are unknown until runtime.

    >Variable variables make your code smell.
    Yep, Get your fresh week-old-road-pizza here!

  • Bu (unregistered)

    I recommend using a base class and then subclassing for additional functionality. You're still using variable variables, but at least you're mapping to a class which can encapsulate any number of functions and properties. In the example above maybe do something like this...

    // base class
    class DBFunction {
    function doSomething($page_id, $position, $target) { }
    }

    class SomeOtherDBFunction extends DBFunction {
    function doSomething($page_id, $position, $target) {
    // specific stuff here.
    }
    }

    class YetAnotherDBFunction extends DBFunction {
    function doSomething($page_id, $position, $target) {
    // specific stuff here.
    }
    }

    // then instead of this...
    // if ($admin) {return $admin($page_id, $position, $target);}

    // ..do something like this...
    if($admin) {
    $dbCmd = new $admin; // variable variable
    $dbCmd->doSomething($page_id, $position, $target);
    }

    --------

    This way, you can see the function being called. However, you still don't know what class is being instantiated without printing out the class name.

  • Rob Menke (unregistered)

    Re Bu's suggestion:

    I'd prefer to do it that way myself, in that it makes the object-nature of the data more explicit. However, you'd probably want to put the tuple in the constructor itself rather than passing the values in as arguments to the method.

    The only reason you may want to dispatch across the method name rather than creating a class is for performance reasons. Note that you're creating and subsequently destroying a user object in that 'if' block, which is not a cheap operation by any means. As Ian pointed out, prefixing the retrieved name with a constant string would prevent the majority of trojan attacks. On the database side, triggers could limit the method names in the tables to prevent errors.

    Now, if you were doing this in Perl and the database row were loaded into some sort of reference, you could quickly create a polymorphic object via 'bless.' But blessing outside of constructors is probably as much an anathema to this group as the idea of variable variables.

    And just to annoy everybody a little more, the technique of embedding method names in your database rows is endorsed by none less than the Shlaer/Mellor school of OOD/OOP (at least it was in 1996, when I took their course in OOD). So it's not exactly a fringe technique.

  • Waigl (unregistered)
    Alex Papadimoulis:
    Okay, that's enough PHP for a while now :-D. Don't worry, if you're really itching for more, Benson Wong recently launched a site just for such oddities: ThePhpWtf.com. If its been at least twenty minutes since you've eaten, then go check it out!

    Unfortunately, this site seems to have fallen victim to a domain grabber...

  • tbo (unregistered)

    class SonicWall { ... }

    class LinkSys { ... }

    class NetGear { ... }

    $row = sql("SELECT device_type FROM router"); $type = $row['device_type'];

    $device = new $$type();

    It's been a while since I worked on this particular project, but I remember doing something like that.

Leave a comment on “$_PHP["admin"]”

Log In or post as a guest

Replying to comment #:

« Return to Article