Is it a bad practice to use EXIT WHEN instruction

Question

It may sound like a silly question, but I hope I'll make myself clear enough.

1. When talking about Spaghetti Code, the basis of it is the usage of GOTOs. I had a peer that was used to say if I put a breakpoint at the end of the code and this breakpoint isn't reached everytime, something is wrong.
2. Nevertheless, is a common (and I'd say, a rule) to use EXIT WHEN structures within Oracle packages (usually followed by a %NOTFOUND test).

Taking for granted that using EXIT breaks the programming flow, isn't something that doesn't match between 1 and 2?

Is everyone programming in PL/SQL following a bad practice? Does PL/SQL don't follow this specific pattern for conditionals?

Is there any performance reason under Oracle's hood to use such statements?

Apologies if this question has been already asked, I couldn't find anything similar around.

Answer 1

It´is very recommendable to keep it simple your code, so I can tell you what PL/SQL guru says about it:

NOTE : In some cases is not recommendable use of CURSOR-FOR-LOOP. You can consider one more intelligent manner of choose single SELECT-INTO or BULK COLLECT according of your needs.

Source : On Cursor FOR Loops, Oracle Magazine By Steven Feuerstein

(Reference: Feuerstein, TOP TWENTY PL/SQL TIPS AND TECHNIQUES):

Loops

12. Take advantage of the cursor FOR loop.

The cursor FOR loop is one of my favorite PL/SQL constructs. It leverages fully the tight and effective integration of the procedural aspects of the language with the power of the SQL database language. It reduces the volume of code you need to write to fetch data from a cursor. It greatly lessens the chance of introducing loop errors in your programming - and loops are one of the more error-prone parts of a program. Does this loop sound too good to be true? Well, it isn’t - it’s all true!

Suppose I need to update the bills for all pets staying in my pet hotel, the Share-a-Din-Din Inn. The example below contains an anonymous block that uses a cursor, occupancy_cur, to select the room number and pet ID number for all occupants at the Inn. The procedure update_bill adds any new changes to that pet’s room charges.

DECLARE
   CURSOR occupancy_cur IS
    SELECT pet_id, room_number
    FROM occupancy
    WHERE occupied_dt = SYSDATE;
   occupancy_rec occupancy_cur%ROWTYPE;
BEGIN
    OPEN occupancy_cur;
    LOOP
    FETCH occupancy_cur
      INTO occupancy_rec;
      EXIT WHEN occupancy_cur%NOTFOUND;
      update_bill
      (occupancy_rec.pet_id,
      occupancy_rec.room_number);
    END LOOP;
    CLOSE occupancy_cur;
END;

This code leaves nothing to the imagination. In addition to defining the cursor (line 2), you must explicitly declare the record for the cursor (line 5), open the cursor (line 7), start up an infinite loop, fetch a row from the cursor set into the record (line 9), check for an end-ofdata condition with the cursor attribute (line 10), and finally perform the update. When you are all done, you have to remember to close the cursor (line 14). If I convert this PL/SQL block to use a cursor FOR loop, then I all I have is:

DECLARE
CURSOR occupancy_cur IS
  SELECT pet_id, room_number
  FROM occupancy WHERE occupied_dt =
  SYSDATE;
BEGIN
  FOR occupancy_rec IN occupancy_cur
  LOOP
    update_bill (occupancy_rec.pet_id,
    occupancy_rec.room_number);
  END LOOP;
END;

Here you see the beautiful simplicity of the cursor FOR loop! Gone is the declaration of the record. Gone are the OPEN, FETCH, and CLOSE statements. Gone is need to check the %FOUND attribute. Gone are the worries of getting everything right. Instead, you say to PL/SQL, in effect:: ÒYou and I both know that I want each row and I want to dump that row into a record that matches the cursor. Take care of that for me, will you?" And PL/SQL does take care of it, just the way any modern programming language integrated with SQL should.

Answer 2

Yes, many people are following a bad practice.

Bad Style

I agree with @Osy that OPEN/FETCH/CLOSE adds completely unnecessary code. I would go even further, and say that you should almost never use CURSOR.

First of all, you normally want to do as much as possible in plain SQL. If you need to use PL/SQL, use an implicit cursor. It will save you a line of code and will help you keep related logic closer together.

I'm a strong believer in keeping individual units of code as small as possible. At first glance, it seems like a CURSOR can help you do this. You can define your SQL up top in one place, and then do the PL/SQL looping later.

But in reality, that extra layer of indirection is almost never worth it. Sometimes a lot of logic is in SQL, and sometimes a lot of logic is in PL/SQL. But in practice, it rarely makes sense to put a lot of complex logic in both. Your code usually ends up looking like one of these:

for records in (<simple SQL>) loop
    <complex PL/SQL>
end loop;

or:

for records in
(
    <complex SQL>
) loop
    <simple PL/SQL>;
end loop;

Either way, one of your code sections will be very small. The complexity of separating those two sections of code is greater than the complexity of a larger, single section of code. (But that is obviously my opinion.)

Bad Performance

There are significant performance implications with using OPEN/FETCH/CLOSE. That method is much slower than using a cursor for loop or an implicit cursor.

The compiler can automatically use bulk collect in some for loops. But, to quote from the Oracle presentation "PL/SQL Performance—Debunking the Myths", page 122:

Don’t throw this chance away by using the open, fetch loop, close form

Here's a quick example:

--Sample data
create table t(a number, b number);
insert into t select level, level from dual connect by level <= 100000;
commit;

--OPEN/FETCH/CLOSE
--1.5 seconds
declare
    cursor test_cur is
    select a, b from t;
    test_rec test_cur%rowtype;
    counter number;
begin
    open test_cur;
    loop
        fetch test_cur into test_rec;
        exit when test_cur%notfound;
        counter := counter + 1;
    end loop;
    close test_cur;
end;
/

--Implicit cursor
--0.2 seconds
declare
    counter number;
begin
    for test_rec in (select a, b from t) loop
        counter := counter + 1;
    end loop;
end;
/