Chacham's Journal: Mini rant: Stupid code

Not only does this code have stupid constants, it has stupid code:

BEGIN
                SELECT NVL(MAX(XXX_BATCH_JOB_R),0) INTO nBatchNo FROM XXX_BATCH_JOB_CONTROL WHERE XXX_BATCH_JOB_N = sBatchName;
            IF nBatchNo = 0 THEN
                nBatchNo := 0;
            END IF;

        EXCEPTION
                WHEN NO_DATA_FOUND THEN
                        nBatchNo := 0;
        END;
              preBatchNo := nBatchNo;
              nBatchNo := nBatchNo + 1;

8 similar references to these variables:

LPAD (NVL (preBatchNo,0),6,0) ||
LPAD (NVL (nBatchNo,0),6,0)

Some_Func(sBatchName, nBatchNo, v_updtPgm, nretcode);

There are no other references to these variables.

What is wrong with this code? Let me count the ways:

1. The IF statement checks the 0 condition, even though 0 will work just fine.
2. If it equals 0, it sets it to 0. Why?
3. An EXCEPTION clause is used.
4. The EXCEPTION is impossible, as MAX() is an aggregate function, and aggregate functions always return data

5. .

6. The return is stored in one variable then immediately moved to another variable.
7. The new new variable has a non-sensical name.
8. The new variable breaks the Hungarian notation used elsewhere.
9. The new variable is used eight times with formatting, and no where else. But the formatting is not done in the variable tiself.
10. The new variable is always used with the old variable, for formatted output, yet they are kept separate.
11. Both variables are NVL()ed over and over again, even though that was handled at the beginning.

There are other issues, such as the lack of a SEQUENCE to truly have unique numbers. But, those are trivial compared to the absolute stupidity of the block itself.

So, how should that code have been written?

SELECT NVL(MAX(XXX_BATCH_JOB_R), 0) INTO nBatchNo FROM XXX_BATCH_JOB_CONTROL WHERE XXX_BATCH_JOB_N = sBatchName;
preBatchNo := LPAD(preBatchNo, 6, 0) || LPAD(nBatchNo + 1, 6, 0)

Between the block and the NVL()s, i feel that the original coder was a blockhead that added no value at all.

Reminiscent of posts on http://thedailywtf.com/

[insanecarbonbasedlif]

Definitely reminds me of posts I've seen on http://thedailywtf.com/ [thedailywtf.com], though I have to admit I have made my fair share of bad contributions due to mid task restructuring or time or ignorance, so I always keep that in mind when laughing at the terrible code I run across.

Re:

[Chacham]

Heh. But this stuff should be really, really basic.

On another note, Going through the effort to write it up here helps me release the want to scream.