Wednesday, December 13, 2006

Code Re-use

This code was anonymised by the sender and some of the original logic may be a bit harder to make sense of as a result. Instead of EMP records, you should probably imagine it checking billion-dollar financial deals or nuclear reactor core temperature readings. Anyway, I think I've figured out what it's supposed to do.

You pass in an EMP record containing first name, last name, email address and so on, and it calls OTHER_PROC(email_address, the_emp_record_as_passed_in) - but only if the email address is not null, and it is unique for employees with that name. For some reason. But how to check? Simple:

  1. Use a cursor to load all the rows for that name into an array. (Apparently there were 130+ columns in the original table.)
  2. Check array.COUNT to see how many rows there are.
  3. If there is only one row, use it, taking care to use an NVL expression because we definitely don't want a NULL email address.
  4. If there is more than one row, open the same cursor again, and this time loop through it comparing each row's email address with the previous one. If it's the same, set lv_email_same = 1, otherwise set it to 0. That way, at the end of the loop we'll know whether they were all the same or not, right?

PROCEDURE unleash_havoc (p_emp_rec emp%rowtype)
IS
   lv_email_same  NUMBER(1) :=0; -- 0: no, 1: yes
   lv_email_null  NUMBER(1) :=0; -- 0: no, 1: yes
   lv_email       emp.email%TYPE := NULL;
   ln_row         NUMBER;

   TYPE emp_tab IS TABLE OF emp%ROWTYPE;
   lt_emp_data  emp_tab;

   CURSOR c_emp (p_last_name VARCHAR2, p_first_name VARCHAR2) IS
      SELECT *
      FROM   emp e
      WHERE  e.last_name = p_last_name
      AND    e.first_name = p_first_name;

BEGIN
   OPEN c_emp (p_emp_rec.last_name, p_emp_rec.first_name);
   FETCH c_emp BULK COLLECT INTO lt_emp_data;
   CLOSE c_emp;

   IF lt_emp_data.COUNT = 1 THEN
       ln_row := lt_emp_data.FIRST;
       lv_email  := NVL(lt_emp_data(ln_row).email,NULL);
       other_proc(lv_email,p_emp_rec);

   ELSIF lt_emp_data.COUNT > 1 THEN

       FOR r IN c_emp (p_emp_rec.last_name, p_emp_rec.first_name) LOOP
           IF NVL(r.email,'X') = NVL(lv_email,'X') THEN
               lv_email := r.email;
               lv_email_same := 1;
           ELSE
               lv_email := r.email;
               lv_email_same := 0;
           END IF;

           IF r.email IS NULL THEN
               lv_email_null := 1;
           ELSE
               lv_email_null := 0;
           END IF;
       END LOOP;

       IF  lv_email_same = 1
       AND lv_email_null = 0
       THEN
           lv_email  := NVL(lt_emp_data(ln_row).email,NULL);
           other_proc(lv_email,p_emp_rec);
       ELSE
           ...
       END IF;
   END IF;
END;

6 comments:

Scott Swank said...

We can safely assume that the author never considered the consequences of the database supporting more that one user at a time -- or at least no more than ones user calling this code at a time.

I'll sleep well tonight, safe in the knowledge that at least this employer needs me.

gandolf989 said...

Isn't there some way that the author of this code could use more explicit cursors, and always remember to the leave the cursor open, you know for the next time you call this code. : (

Anonymous said...

Scott said: "I'll sleep well tonight, safe in the knowledge that at least this employer needs me. "

There is a difference of course between an employer needing you, and an employer realising that they need you.

Noons said...

may God have pity on IT!....

Anonymous said...

You know how the UNIX admins' tagline is "Go away, or I will replace you with a very small shell script" ? Perhaps the (competent) DBA's tagline should be "I will replace you with a very small SQL statement".

Anonymous said...

And I will modify sql.bsq to actually have "truncate database" for those senior dba moments.