VF=>user  ID=>    Login Feedback FAQ Blog

Actions


  On Oracle PL/SQL: Write No Unnecessary Code (Oracle PL/SQL)      Add to Favorites

Summary

It's not hard to write overly elaborate algorithms that only accidentally give you the right answer. Keep your code simple and straightforward, and leverage SQL first and foremost!

Details

Assumes Oracle Database 10g Release 2 or higher
Intermediate Quiz, IDs: 19346/1867068/1224588

Reviewer(s): none

The Question

I execute these statements:

CREATE TABLE plch_animals
(
   animal_id     INTEGER PRIMARY KEY,
   animal_name   VARCHAR2 (100)
)
/

BEGIN
   INSERT INTO plch_animals (animal_id, animal_name)
        VALUES (1, 'Bonobo');

   INSERT INTO plch_animals (animal_id, animal_name)
        VALUES (2, 'Cockatoo');

   INSERT INTO plch_animals (animal_id, animal_name)
        VALUES (3, 'Spider');

   COMMIT;
END;
/

Which of the choices display the following text after execution?

Animals in Alphabetical Order
Bonobo
Cockatoo
Spider

The Choices [↑]

Explanation of Result Icons
Choice 1 (34303) [Do Not Use]
DECLARE
   l_count   INTEGER;
   l_name    plch_animals.animal_name%TYPE;
BEGIN
   DBMS_OUTPUT.put_line ('Animals in Alphabetical Order');

   SELECT COUNT (*) INTO l_count FROM plch_animals;

   FOR indx IN 1 .. l_count
   LOOP
      SELECT animal_name
        INTO l_name
        FROM plch_animals
       WHERE animal_id = indx;

      DBMS_OUTPUT.put_line (l_name);
   END LOOP;
END;
/

This choice is correct, but only because the animal IDs happen to ascend in exactly the same sequence as an alphabetical ordering of the animal names.

You certainly do not want to ever assume this will be the case with real data. Even if you look at the data today and can confirm that pattern. That's just for today.

A warning sign that this code is problematic is the SELECT COUNT(*). You should generally not need to do a separate query just to control the number of iterations of a loop. A cursor FOR loop will usually take care of that for you.




83%
Choice 2 (34304) [Do Not Use]
BEGIN
   DBMS_OUTPUT.put_line ('Animals in Alphabetical Order');

   FOR rec IN (  SELECT animal_name
                   FROM plch_animals
               ORDER BY animal_id)
   LOOP
      DBMS_OUTPUT.put_line (rec.animal_name);
   END LOOP;
END;
/

The vast majority of the work is done within SQL, both in terms of identifying the data to be displayed, and ordering it properly. That's the way to approach implementing any requirement in PL/SQL. First, identify what can be done in SQL. Then finish up in PL/SQL (if anything else is needed).

Having said that, we do NOT recommend this choice because it is only accidentally correct. Notice that I incorrectly ORDER BY the ID, not the name. It just so happens that the data correlates that way and the text is displayed as desired.

I offered this choice as a way of saying: when you test, REALLY TEST. Thoroughly, with lots of different sets of data, positive and negative, etc.




93%
Choice 3 (34305) [Do Not Use]
DECLARE
   TYPE animal_ids_t IS TABLE OF plch_animals.animal_id%TYPE;

   l_animal_ids   animal_ids_t;
   l_name         plch_animals.animal_name%TYPE;
BEGIN
   DBMS_OUTPUT.put_line ('Animals in Alphabetical Order');

   SELECT animal_id
     BULK COLLECT INTO l_animal_ids
     FROM plch_animals;

   FOR indx IN 1 .. l_animal_ids.COUNT
   LOOP
      SELECT animal_name
        INTO l_name
        FROM plch_animals
       WHERE animal_id = indx;

      DBMS_OUTPUT.put_line (l_name);
   END LOOP;
END;
/

This choice is similar to #1, in terms of the basic, flawed assumptions (the animal IDs happen to ascend in exactly the same sequence as an alphabetical ordering of the animal names).

But it's even worse because I fetch all the animal IDs via BULK COLLECT into an array, then when I loop through the array, I execute a single row fetch to get the name.

And then it's even worse worse because the only reason this works is that the ID values happen to align with names in alphabetical order. So the results "look" OK when the test is run with this pathetic set of test data, but in the real world? Ugh.

That's a waste of code, PGA memory and CPU cycles.




79%
Choice 4 (34306)
DECLARE
   TYPE animal_ids_t IS TABLE OF plch_animals.animal_id%TYPE
      INDEX BY plch_animals.animal_name%TYPE;

   l_animal_ids   animal_ids_t;
   l_index        plch_animals.animal_name%TYPE;
   l_name         plch_animals.animal_name%TYPE;
BEGIN
   DBMS_OUTPUT.put_line ('Animals in Alphabetical Order');

   FOR rec IN (  SELECT *
                   FROM plch_animals
               ORDER BY animal_name DESC)
   LOOP
      l_animal_ids (rec.animal_name) := rec.animal_id;
   END LOOP;

   l_index := l_animal_ids.FIRST;

   WHILE l_index IS NOT NULL
   LOOP
      DBMS_OUTPUT.put_line (l_animal_ids (l_index));

      l_index := l_animal_ids.NEXT (l_index);
   END LOOP;
END;
/

Oh my, what a mess. First of all, it's wrong because buried inside the loop is the surprise that I am displaying the IDs instead of the names. So that's that, for this choice. But you may not even have gotten that far. You might have stopped right here

ORDER BY animal_name DESC

Interestingly, that judgement would be wrong, because inside that first loop, I assign the IDs to a string-indexed array, with indexes set to the animal name.

So, in fact, the data in l_animal_ids is ordered properly. But it's a whole lot of code and complicated logic that you do not need at all.




61%
Choice 5 (34307) [Do Not Use]
DECLARE
   l_count   INTEGER;

   TYPE animal_names_t IS TABLE OF plch_animals.animal_name%TYPE;
   l_animal_names animal_names_t;
BEGIN
   DBMS_OUTPUT.put_line ('Animals in Alphabetical Order');

     SELECT COUNT (*)
       INTO l_count
       FROM plch_animals
   ORDER BY animal_name;

     SELECT animal_name
       BULK COLLECT INTO l_animal_names
       FROM plch_animals
   ORDER BY animal_name;

   FOR indx IN 1 .. l_count
   LOOP
      DBMS_OUTPUT.put_line (l_animal_names (indx));
   END LOOP;
END;
/

If there was only a single SELECT (the BULK COLLECT), this wouldn't be so bad, though choice #2 with a cursor FOR loop would be better.

But first I find the number of rows that I will be fetching, and then use that (l_count) to control the execution of the display loop. Totally unnecessary. I could simply have written:

FOR indx IN 1 .. l_animal_ids.COUNT



80%

Answer [↑]

Program units that contain unnecessary code (code that can be removed without changing the behavior of that program) are units inviting maintenance problems. You should always be careful to remove any code that is not needed.

Also consider using the compile-time warning to help you identify at least some of these occurrences. Here is an example of doing to to identify code that could be removed (or requiring some other change in the program unit):

ALTER SESSION SET plsql_warnings = 'enable:all'
/

CREATE OR REPLACE PROCEDURE plch_plw6006
   AUTHID DEFINER
AS
   l_var   NUMBER;

   PROCEDURE not_used
   IS
   BEGIN
      NULL;
   END not_used;
BEGIN
   DBMS_OUTPUT.put_line (l_var);
END plch_plw6006;
/

SHOW ERRORS PROCEDURE plch_plw6006

We should, of course, be looking at a much simpler solution. 

1. Pure SQL of course does the job nicely:

SELECT animal_name FROM plch_animals
 ORDER BY animal_name

2. Or a simple cursor FOR loop:

BEGIN
   DBMS_OUTPUT.put_line ('Animals in Alphabetical Order');

   FOR rec IN (SELECT animal_name FROM plch_animals
                ORDER BY animal_name)
   LOOP
      DBMS_OUTPUT.put_line (rec.animal_name);
   END LOOP;
END;

Statistics and Rankings [↑]

Rank Percentile Score Seconds% Correct
Median
75
50
338
186
80
Best
1
100
478
66
100

Click to View Verification Code [↑]

Resources [↑]

No resources have been provided.

Quiz Link and Tweet [↑]

Pass along a link the quiz or use our text to tweet about this quiz. Thanks for helping to spread the word!

 
About Oracle | Terms of Use | Your Privacy Rights | Copyright 2010-2017, Oracle Corporation