Catch up on stories from the past week (and beyond) at the Slashdot story archive

 



Forgot your password?
typodupeerror
User Journal

Chacham's Journal: Mini rant: Stupid code 2

Journal by Chacham

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.

This discussion has been archived. No new comments can be posted.

Mini rant: Stupid code

Comments Filter:

Logic is a systematic method of coming to the wrong conclusion with confidence.

Working...