Saturday, September 22, 2007

Wrestling with pure eeevveeeel

Jeremy Miller has a whiny rant on a project he's working on.

The whiny part isn't from me, it's from his comments section.

I've been wrestling with pure evil too. It's not java, and it's a whole program that's too long, not a function. there's only one function in there, it's called doublenum, and it doesn't arithmetically double its parameter (which would be at least descriptive), instead it doubles its LENGTH. sometimes. by prepending a zero if it's only one digit wide.

so, applying some items from Jeremy's rant,


  1. Very long method [check, although it's not even a method, it's just PHP code that's not in a function, so everything is global]

  2. Variables are reused for different things throughout the method [check, but at least variable names aren't totally bogus, although it does use a class prepended with the initials of its author]

  3. Lots of if/then exception cases [check - in fact, since there are about 8 common cases, the whole program is bunch of nested if/thens with one code path being one of the common cases, with exactly the same actions copy-pasted all over the place, plus deeply nested if/then exception cases peppered through the code (copy-pasted again), and at the end, just one of those common cases will be executed]

  4. Deeply nested Arrowhead code [ check, except its several arrowheads ]

  5. All data stored in Hashtable's -- and it's this data that's getting bamboozled somewhere [check - plus parallel PHP arrays (integer indexed), plus parallel (sort of) associative arrays where the keys are concatenated strings of the subkeys, plus some two dimensional arrays where the concatenated string keys previously mentioned are split up and make up the two dimensions]

  6. Numbers that come out wrong - [ check, or, actually, a really obscure bug where orders get their order quantity zeroed ]



And for extra bogus points:

  1. $r=7-($cday-$x);
    $mt = sprintf("%2d",chop(`date +%m --date '$r days' `));
    $dy = sprintf("%2d",chop(`date +%d --date '$r days' `));
    $yr = sprintf("%2d",chop(`date +%Y --date '$r days' `));

    what moron calls a system function THREE TIMES to get the pieces of a date? If they didn't know anything about time functions in php (or perl, this crap is all over the perl code these people wrote too), they could at least have called date once, and then extracted the fields out of there. But then, if they were too dumb to learn how to use the time functions (or even just to learn that they exist), I suppose it's too much to expect that they would figure out, The function names aren't helpful, but that's not as much a bogosity as forking to get the time, three times. Even selecting the date from the database would be better than this stupidity, and if they couldn't wrap their brains around that, at least fork only once!,

    date +%m-%d-%Y --date '1 day'

  2. oh, and that doublenum function? The only function defined in the whole program? It's actually inline in the code TWICE. But php doesn't do static analysis worth anything. Instead, if the two function definitions are in perfectly separate code-paths (it's never possible for both function definitions to be executed in the same run), then PHP sees no problem with that. PHP only points out function redefinition errors when the function definition is actually executed, e.g., no error here:

    function f()
    {
    return 0;
    }

    if(false)
    {
    function f()
    {
    return 31415;

    }

    }

My mistake with this abortion is that I've been trying to live with it, fixing it incrementally, when fixes needed to be done. I should have thrown this away long ago though, and just rewritten it so that it wouldn't be so bogus.

I was avoiding doing that because the horrendous date logic was totally obscured by the implementation, so it wasn't possible to extract what the date logic was supposed to do at all just from the source code (not possible for me anyway, I would get angry too early and give up). Now I'm going to have to understand the date logic and rewrite it all. Or if that can't be done, then use the current horrendous implementation as a test for proving that my implementation is "correct" (or at least complies with the current implementation, since I can't understand the current implementation, I'm not currently capable of proving *IT* correct. I can only take it as a given).

I'd better not meet any of the previous implementors in this or any future world. They're going to KNOW what I think of them, and then I'll hire them so I can fire them one second after.

No comments: