Thursday, October 13, 2016

Another reminder of the elegance and brevity of CASE expressions

Building a script for an upcoming PL/SQL Challenge quiz, I wrote a nested procedure as follows:

   PROCEDURE show_cursor_status
   IS
   BEGIN
      IF all_in_one_cur%ISOPEN
      THEN
         DBMS_OUTPUT.put_line ('all_in_one_cur is still open');
      ELSE
         DBMS_OUTPUT.put_line ('all_in_one_cur is closed');
      END IF;

      IF department_cur%ISOPEN
      THEN
         DBMS_OUTPUT.put_line ('department_cur is still open');
      ELSE
         DBMS_OUTPUT.put_line ('department_cur is closed');
      END IF;

      IF employee_cur%ISOPEN
      THEN
         DBMS_OUTPUT.put_line ('employee_cur is still open');
      ELSE
         DBMS_OUTPUT.put_line ('employee_cur is closed');
      END IF;
   END;

Nothing wrong with that, of course. Works just fine.

But there's a lot of repetition. I hate that. I like to normalize my code. And when the repetitive code is based on an IF statement, I immediately think of CASE - expressions:

   PROCEDURE show_cursor_status
   IS
   BEGIN
      DBMS_OUTPUT.put_line (
            'all_in_one_cur is '
         || CASE WHEN all_in_one_cur%ISOPEN THEN 'open' ELSE 'closed' END);

      DBMS_OUTPUT.put_line (
            'department_cur is '
         || CASE WHEN department_cur%ISOPEN THEN 'open' ELSE 'closed' END);

      DBMS_OUTPUT.put_line (
            'employee_cur is '
         || CASE WHEN employee_cur%ISOPEN THEN 'open' ELSE 'closed' END);
   END;

But, wait, still....there's more repetition: each of those CASE expressions. So I can use a nested function to encapsulate all of that, and I am left with:

   PROCEDURE show_cursor_status
   IS
      FUNCTION cursor_state (isopen_in IN BOOLEAN)
         RETURN VARCHAR2
      IS
      BEGIN
         RETURN CASE WHEN isopen_in THEN 'open' ELSE 'closed' END;
      END;
   BEGIN
      DBMS_OUTPUT.put_line (
         'all_in_one_cur is ' || cursor_state (all_in_one_cur%ISOPEN));

      DBMS_OUTPUT.put_line (
         'department_cur is ' || cursor_state (department_cur%ISOPEN));

      DBMS_OUTPUT.put_line (
         'employee_cur is ' || cursor_state (employee_cur%ISOPEN));
   END;

Ah....much better. No code repetition. Of course, in a subprogram this small, repetition is not too deadly a problem. Still, programs tend to get more complex over time, not simpler.

So assuming you (I) will be coming back to this code in the future, making sure that any particular piece of logic is implemented in just once place greatly reduces the cost and complexity of maintenance going forward, and also reduces the chance of introducing bugs.

Follow Up

I invited readers (via Twitter) to improve upon my code, and BluShadow came through with:

PROCEDURE show_cursor_status
IS
   PROCEDURE cursor_state (cur_name IN VARCHAR2, isopen_in IN BOOLEAN)
   IS
   BEGIN
      DBMS_OUTPUT.put_line (
            cur_name
         || ' is '
         || CASE WHEN isopen_in THEN 'open' ELSE 'closed' END);
   END;
BEGIN
   cursor_state ('all_in_one_cur', all_in_one_cur%ISOPEN);
   cursor_state ('department_cur', department_cur%ISOPEN);
   cursor_state ('employee_cur', employee_cur%ISOPEN);
END;

5 comments:

  1. Great to know that we can use CASE in dbms_output

    ReplyDelete
    Replies
    1. You bet!

      DBMS_OUTPUT.put_line accepts a string as its only argument. So you can pass to this built-in any expression that evaluates to (or can be implicitly converted to) a string.

      That certainly includes a CASE expression.

      Delete
  2. Still too much repetition... I'd go for:

    PROCEDURE show_cursor_status (str IN VARCHAR2)
    IS
    PROCEDURE cursor_state (cur_name IN VARCHAR2, isopen_in IN BOOLEAN)
    IS
    BEGIN
    DBMS_OUTPUT.PUT_LINE(cur_name||' is '|| CASE WHEN isopen_in THEN 'open' ELSE 'closed' END);
    END;
    BEGIN
    cursor_state('all_in_one_cur', all_in_one_cur%ISOPEN);
    cursor_state('department_cur', department_cur%ISOPEN);
    cursor_state('employee_cur', employee_cur%ISOPEN);
    END;

    ;)

    ReplyDelete
  3. I'm wondering why you went from:

    PROCEDURE show_cursor_status IS

    to:

    PROCEDURE show_cursor_status (str IN VARCHAR2) IS

    The added unused parameter will break any code that was using the original spec.

    ReplyDelete
  4. Thanks for pointing that out, David. I should have removed the str parameter. I pulled this code out of a quiz I was writing, and that was displaying the "context." Not relevant here. Fixing now.

    ReplyDelete