Is this bad or am I just picky?

Jeroen De Dauw

Came across this "class" during a code review. My reaction is pretty much "the fuck is this". Do you think this is good code or not?

class Listalizer {

public static function listalize( $convert, &$data ) {
    if ( is\_object( $data ) ) {
        try {
            $data = call\_user\_func( $convert, $data );
        } catch ( \\Exception $ex ) {
            try {
                trigger\_error( "listalize: Callable " . self::callable2String( $convert )
                    . " caused an exception: " . $ex->getMessage(), E\_USER\_WARNING );
            } catch ( \\Exception $exx ) {
                // Argh! We \*can't\* throw an exception!
                $exx = (object)$exx; // this is just a breakpoint anchor.
            }

            $data = false;
        }
    }

    if ( is\_array( $data ) || $data instanceof \\ArrayObject ) {
        foreach ( $data as $key => &$value ) {
            self::listalize( $convert, $value );
        }
    }
}

public static function objectify( $convert, &$data, $role = null ) {
    if ( is\_array( $data ) ) {
        try {
            $data = call\_user\_func( $convert, $data, $role );
        } catch ( \\Exception $ex ) {
            try {
                trigger\_error( "objectify: Callable " . self::callable2String( $convert )
                    . " caused an exception: " . $ex->getMessage(), E\_USER\_WARNING );
            } catch ( \\Exception $exx ) {
                // Argh! We \*can't\* throw an exception!
                $exx = (object)$exx; // this is just a breakpoint anchor.
            }

            $data = false;
        }
    }

    if ( is\_array( $data ) || $data instanceof \\ArrayObject ) {
        foreach ( $data as $key => &$value ) {
            self::objectify( $convert, $value, $key );
        }
    }
}

public static function callable2String( $callable ) {
    if ( is\_array( $callable ) && count( $callable ) === 1 ) {
        $callable = array\_pop( $callable );
    }

    if ( is\_string( $callable ) ) {
        return $callable;
    } elseif ( is\_object( $callable ) ) {
        return get\_class( $callable ) . '->\_\_invoke';
    } elseif ( is\_array( $callable ) ) {
        $target = $callable\[0\];

Brisingr Aerowing

Ick. That's PHP, right?

Bob Dole

The internet is a great way to get on the net.

:doh: 2.0.82.7292 SP6a

Jeroen De Dauw · :doh: 2.0.82.7292 SP6a

Yep, PHP. Though some people consider this to be a horror on itself, that's not what I am getting at here :)

Jeroen De Dauw (blog | Twitter | Identi.ca)

_beauw_

It has the air of over-architected gibberish about it, but I don't even know what language that is. EDIT: OK, it's PHP. I'd be curious to hear what it enables that wouldn't otherwise be possible. I suspect the answer is "nothing."

fjdiewornncalwe

Could be worse. It could have been vb... :)

I wasn't, now I am, then I won't be anymore.

Ingo

Jeroen De Dauw wrote:

My reaction is pretty much "the f*** is this". Do you think this is good code or not?

Well the try-blocks in the catch-statements are... Well, I don't know. It's not the best code, but I've seen much worse. I think with some arguments this programmer would be able to code readable, efficient code. So there is more than a shimmer of hope. ;)

------------------------------ Author of Primary ROleplaying SysTem How do I take my coffee? Black as midnight on a moonless night. War doesn't determine who's right. War determines who's left.

StM0n

Mhm... lot of ifs and elseifs... and try-catches. Could be more compact...

(yes|no|maybe)*

Lost User

Ok, I found a spelling mistake in the code (an extra 'x')

Jeroen De Dauw wrote:

$exx

jim lahey

Your reaction is the same as mine. Obvious digs at PHP aside, what does this class even do?

S Houghtelin

Too much

$exx

Or not enough...

It was broke, so I fixed it.

Gary Huck

Okay ... what are you getting at?

Lost User

I think this just converts an array to a string. Can be used in those beginner/advanced/pro/guru code examples :)

Scott Burkow

the f***. developer needs more Argh comments. (am I supposed to include an emoticon?)

satovey

After taking a good look see, this class takes an array, converts it to strings and tests to see if the string is a callable object with a method inside. In PHP, you can assign a function or object name to a string and call it as though you you had hard coded the name in the code. This code, despite all the complaints is actually what you would have to do in order to avoid the exceptions that occur when you call a function or object that does not exist. The author is utilizing a class perhaps to keep all the related code in one place, but a simple function would have sufficed.

Bob G Beechey

I am getting tired of silly comments about vb. Presumably by someone who has not checked out vb since vb6.

fjdiewornncalwe

Apparently you missed the Joke icon. I've had to do plenty of vb and vb.net work over the years so I do not speak ignorantly on the matter. :)

I wasn't, now I am, then I won't be anymore.

Bob G Beechey

Damn. I knew I had to get my eyes tested.

Brady Kelly

Last year I translated a whole lot of VB6 code into VB.NET 10. After the project was over he complained that most of my code was unintelligible and "I had used LINQ!"

H Brydon

Bob G Beechey wrote:

I am getting tired of silly comments about vb. Presumably by someone who has not checked out vb since vb6.

I've used VB2 through VB6 and VB.Net (2002) through VB.Net 2005. VB's mother wears army boots!

-- Harvey