I've recently been handed a project that involves fixing some of the most bletcherous code I've seen thus far in my career, apparently written by a professional perl programmer. Here are some examples, posted for you're amusement ;-) 

$content_dem = "$content_dem"."DEM"."\t"."1"."\t"."1"."\n";

[download]
#Remove commas
$content =~ s!, "?!!gis;
@array = split(/"/, $content);

[download]
if (system("/bin/cp $temp_cur $final_file")) {
    printf("Error - file '$final_file' not updated.\n");
    exit(-1);
}

[download]
select STDOUT;
$| = 1;
open SAVEOUT, ">&STDOUT";
close STDOUT;
open STDOUT, ">&SAVEOUT";

[download]

Suffice it to say, I would much rather re-write all of this from scratch than have to live with the thought that anyone else would ever have to try and hack / decipher them. Or end up having to work on them again in a couple of months time.

However, it did get me thinking, what's the worst / most bizarre / inexplicable code my fellow monks have come across over the years? I'm sure I'm probably just scraping the tip of the iceberg here (though, having said that, I must confess that the intention of the writer of the last chunk of code is a mystery to me).

Replies are listed 'Best First'.
RE: The inheritance of Cruft
by Fastolfe (Vicar) on Oct 25, 2000 at 00:59 UTC
    I myself have been guilty in my first few weeks of Perl of doing stuff like: 
    if (condition) {} 
   else { &actually_do_something() }

    [download]
    I did figure out I ought to have done !condition the next day, though it also took me about a year to start getting comfortable with more idiomatic ways to express this: 
    &actually_do_something unless condition;

    [download]
    Most of us have code from very early on in our learning cycles to qualify as pretty funny if not horrible. I'm pretty sure some of my code fitting these categories is still used in production settings, as well...

    I shudder to think what more experienced Perl developers think/say when they come across it. It embarrasses me.

[reply]
[d/l]
[select]
RE: The inheritance of Cruft
by runrig (Abbot) on Oct 25, 2000 at 00:47 UTC
    How about this? Can you say 'use a hash array' or 'use "my", not "local"', or even 'you don't need an index counter in a for loop':
    sub doLookup {
        local($str,$i);
        $str = $_[0];
# Is this an IgnoredWord?
        for ($i = 0; $i < @gIgnoreWords; $i++) {
                if ($str eq $gIgnoreWords[$i]) {
                        return "";
                }
        }
# Is this a ReplacedWord?
        for ($i = 0; $i < @gOldWords; $i++) {
                if ($str eq $gOldWords[$i]) {
                        return $gNewWords[$i];
                }
        }
        return $str;
}

    [download]
[reply]
[d/l]
      Dang! Your predecessor must have been picking the pocket of the person who bequoth me all the code I maintain =)

      Not a foreach (@array) in the lot, mind you. and local() always, never my(). Granted, her code *works*, and she learned Perl on the job.

      Philosophy can be made out of anything. Or less -- Jerry A. Fodor

[reply]
RE: The inheritance of Cruft
by Adam (Vicar) on Oct 25, 2000 at 00:54 UTC
    I've seen worse code then that before.
    My favorite was a block of code 100 lines long that opened 15 file handles, got some data from a sql database, and then wrote different portions of that data to each filehandle (if there was data pertaining to that handle) and then closed the file handles. There was so much code that it took me an hour to figure out what was going on and another hour of sitting in my chair laughing and wondering if the guy ever thought to use a loop. So I re-wrote it in a loop: 10 lines (not counting the sql query).
[reply]
RE: The inheritance of Cruft
by extremely (Priest) on Oct 25, 2000 at 11:24 UTC
    Oh I kept the first +20 line perl script I ever wrote to remind my self how bad it can get... 'twas a set of CGIs with all the greatest voodoo and cargo-cult programming.

    When ever I get cocky I just pull that out and bring myself right back down. This is a nice one: 

    @call_history=$dbh->sql("select * from teamweb..CALLS where Userid='$u
+serid'");

foreach $line (@call_history)
        {
        $havethey=@$line[0];
        }

    [download]

    I was fuzzy on both SQL and arrayref nesting =) And this is from the REWRITE... I have been cured of the evil layout, btw. I haven't done anything bad enough recently to warrant a look at the original. Ahhh, 32kb of linear spaghetti code. =)

    UPDATE: That bit with the STDOUT redirection still bothers me. I wonder if they have a format that they want to apply to STDOUT and that other SAVEOUT. If I read it right, they could use either handle and have one format work. Evil if that is what they are up to, otherwise I'd bet on a cargo-cult version of something similar. Maybe they redirect STDOUT later and just want to keep SAVEOUT around to bring it to life yet again later? Man that bugs me...

    --
    $you = new YOU;
    honk() if $you->love(perl)

[reply]
[d/l]
RE: The inheritance of Cruft
by d_i_r_t_y (Monk) on Oct 25, 2000 at 08:46 UTC
    i've also seen far worse code than that... at least you know what the code does...

    the bottom line for me is that code has to be comprehensible at the end of the day. one-liners are pretty cool when it's throwaway/personal code, but if someone subjects me to having to read their deeply nested, ultra-idiomatic, highly !@##$$%%%^^^ punctuated, poorly thought-out chunk of code with no documentation then i am going to walk over to their desk and kick their butt ;-)

    after all, personal style is personal style, but you can still be funky and clever if you don't use 5 lines instead of one...

    as for the last code-chunk -- they appear to be saving their STDOUT in order to discard the output from something else. if indeed these lines occur as show without something after the 'close STDOUT' step, then i too am confused since they would (seem to) be accomplishing nothing.

[reply]

Back to Meditations