Please create an account to participate in the Slashdot moderation system

 



Forgot your password?
typodupeerror
Get HideMyAss! VPN, PC Mag's Top 10 VPNs of 2016 for 55% off for a Limited Time ×
User Journal

Journal Chacham's Journal: Mini rant: Stupid code 2

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:

Asynchronous inputs are at the root of our race problems. -- D. Winker and F. Prosser

Working...