On the one hand, it’s a little late for New Year’s resolutions. On the other hand, it is never too late to explore ways to improve the quality of our code. And because I am writing this article in January, it sure seems like the right time for me to think about ways to write better PL/SQL code.
Have you ever noticed that a lot of the code you write looks a lot like code you wrote last week or last month? We often follow patterns in our software: write a cursor FOR loop like this, construct a function like that. And because humans are such creatures of habit, we also tend to follow antipatterns: suboptimal ways of writing code.
In this article, I offer up several small-scale antipatterns: the kinds of bad habits that can creep (or carry over from the past) into our code on a line-by-line basis. But I don’t stop there. I then offer patterns that you and the people with whom you work should be happy to follow this year and in the years to come.
Long ago in a database far away, when PL/SQL was just a young language, it did not support all the standard SQL functions such as SYSDATE. So if you needed to get the current system date, you needed to “fall back” on SQL as follows:
DECLARE now DATE; BEGIN SELECT SYSDATE INTO now FROM dual; END;
The need to use SQL to execute SYSDATE has long been a thing of the past.
Many developers also think that when they need to get the next value from a sequence in PL/SQL, they have to write code like this:
DECLARE l_nextkey my_table.my_pky%TYPE; BEGIN SELECT my_table_seq.NEXTVAL INTO l_nextkey FROM dual; END;
Not true! As of Oracle Database 11g, you can now natively call both the NEXTVAL and CURRVAL directly inside PL/SQL, as in
DECLARE l_nextkey my_table.my_pky%TYPE; BEGIN l_nextkey := my_table_seq.NEXTVAL; END;
The bottom line is that if you are using Oracle Database 11g or higher, the days of needing to use SQL to execute functionality not available in PL/SQL are pretty much gone.
If you see a “SELECT FROM dual” in your code, I suggest you ask: “Do I really need that? Can’t I just call it directly in PL/SQL?”
And, of course, it is easy to discover the answer to that question: give it a try!
I like the BOOLEAN datatype. I am glad it is available in PL/SQL and look forward to the day when I can create a column in a table of type BOOLEAN.
I like BOOLEAN because so much code in programs involves conditional logic, such as “If x is true, then y” or “While x is true, execute loop body.”
Without a BOOLEAN datatype, each instance of “x” must be an expression, and that expression can be quite complex, as in
IF (l_employee.salary < c_max_salary AND l_employee.hire_date < ADD_MONTHS (SYSDATE, -60)) OR override_limit_in = 1 THEN
That complexity then leads us to write comments to explain the code. A comment for the above logic might be
/* If employed for more than five years and not yet at maximum salary or user requested override... */
With a BOOLEAN datatype, everything gets clearer. Here’s a rewrite of that complex condition expression:
IS... previous declarations ... c_below_max_salary CONSTANT BOOLEAN := l_employee.salary < c_max_salary; c_employed_at_least_5_years CONSTANT BOOLEAN := l_employee.hire_date < ADD_MONTHS (SYSDATE, -60)); c_override_flag CONSTANT BOOLEAN := override_limit_in = 1; BEGIN IF (c_below_max_salary AND c_employed_at_least_5_years) OR c_override_flag = TRUE THEN
So one antipattern to watch out for is the complicated expression embedded inside an IF statement. Instead, use Boolean variables and constants to hide the details behind a name.
There is, though, a second Boolean antipattern lurking in that rewritten code:
c_override_flag = TRUE
All too often, I come across code that looks like
IF l_flag = TRUE
CASE WHEN l_at_min_balance = FALSE
WHILE (l_found_it <> TRUE) LOOP
This code is valid, but it is not as intuitive as it could be and it reflects a misunderstanding of Booleans.
Here’s the thing: a Boolean can be only TRUE or FALSE (or, in the world of Oracle Database, TRUE, FALSE, or NULL). So there’s no reason to write “IF boolean_var = TRUE”. You can just write “IF boolean_var”. And then you will quickly discover whether you named your variable properly.
Consider the IF statement that contained Boolean constants:
IF (c_below_max_salary AND c_employed_at_least_5_years) OR c_override_flag = TRUE THEN
I argue that the “= TRUE” is entirely unnecessary. That expression could simply be
IF (c_below_max_salary AND c_employed_at_least_5_years) OR c_override_flag THEN
OK, so there’s a little bit less code, and that’s almost always a good thing. But now when I read that expression, it doesn’t sound right: “If this employee is below the maximum salary and started more than 5 years, or override flag.”
Whenever you declare anything, you want to make sure the name describes as accurately as possible the value that thing will hold (if it’s a variable) or represents (such as a type). That is especially true for Booleans.
My constant, c_override_flag, is a flag—an indicator of the state of the override request. But the name of the constant does not reveal that state: was the override requested or not? A small change in the name removes that ambiguity, resulting in more-readable code:
IF (c_below_max_salary AND c_employed_at_least_5_years) OR c_override_requested THEN
So remember: a Boolean can be only TRUE, FALSE, or NULL. Don’t write, “IF my_boolean IS TRUE”. Just write, “IF my_boolean”. And then make sure you use descriptive names for your constants and variables so that the code tells its story smoothly and clearly.
By the way, Boolean is capitalized because that datatype is named after George Boole, the father of symbolic logic (more information on Boole is available at wikipedia.org/wiki/George_Boole).
If something goes wrong when you execute your code, the PL/SQL runtime engine will raise an exception. You can then choose to handle that exception in your block or let it propagate out to the enclosing block.
Or if you came to PL/SQL from another language that did not offer a similar exception model, you might write code like this:
FUNCTION total_salary ( total_out OUT NUMBER) RETURN PLS_INTEGER IS l_total NUMBER; BEGIN... some_code ... l_total := total_out; RETURN 1; EXCEPTION WHEN OTHERS THEN RETURN 0; END;
You might then call this function:
BEGIN l_status := total_salary (l_total); IF l_status = 1 THEN ... continue ... END IF;
Yes, you can write PL/SQL code this way, but it is a bad idea, for multiple reasons:
And that approach to exception handling is certainly not necessary in this case.
Use the built-in exception raising and handling framework of PL/SQL. If you do not need to trap an exception, let it propagate out unhandled. If you need to implement special logic for specific errors, then write a handler just for that:
FUNCTION total_salary RETURN NUMBER RETURN PLS_INTEGER IS l_total NUMBER; BEGIN ... some_code ... RETURN l_total; EXCEPTION WHEN ZERO_DIVIDE THEN RETURN 0; END;
Then you leave it to the invokers of your function to decide what to do about exceptions. If they choose to do nothing special (the equivalent of forgetting to check the status code after calling the first version of total_salary), no problem. If an exception propagates out of total_salary, the invoking block will be terminated by PL/SQL.
The resulting code is simpler, easier to maintain, and far easier to support.
BEGIN l_total := total_salary (); ... continue ... EXCEPTION WHEN OTHERS THEN log_error; END;
One of the mantras of structured programming is, “One way in, one way out.”
For example, there should be just one way into a loop or a function and just one way to leave that loop or function.
In PL/SQL there is generally only one way to go into or start a chunk of code: you call the function, and in you go. You start the loop, and in you go. But it is all too easy to write loops with multiple termination paths. It is also certainly possible, and all too common, to write functions with multiple RETURN statements.
Here’s an example:
FUNCTION status_desc ( cd_in IN VARCHAR2) RETURN VARCHAR2 IS BEGIN IF cd_in = 'C' THEN RETURN 'CLOSED'; ELSIF cd_in = 'O' THEN RETURN 'OPEN'; ELSIF cd_in = 'A' THEN RETURN 'ACTIVE'; ELSIF cd_in = 'I' THEN RETURN 'INACTIVE'; END IF; END;
The problem with having multiple ways out is that it makes the debugging process much harder. I call my function. It returns the wrong value. OK, but which RETURN returned the wrong value? Which logic branch did it follow? And if I want to trace (write to a log table) what my function is doing, I have to insert trace code before each return. Finally, with multiple RETURNs, it is more likely that there will be a pathway that does not execute any RETURN statement. The following demonstrates the result when I call the status_desc function with an undefined code:
BEGIN DBMS_OUTPUT.PUT_LINE ( status_desc ('X')); END; / ORA-06503: PL/SQL: Function returned without value
This is an embarrassing error for a user to have to report. It sends a clear message that I designed and tested my code very poorly.
Fortunately, it is quite easy to fix this antipattern: limit yourself to just one RETURN in the executable section of the function, and make it the very last statement in that section.
I rewrote the status_desc function to follow this approach:
FUNCTION status_desc ( cd_in IN VARCHAR2) RETURN VARCHAR2 IS BEGIN RETURN CASE cd_in WHEN 'C' THEN 'CLOSED' WHEN 'O' THEN 'OPEN' WHEN 'A' THEN 'ACTIVE' WHEN 'I' THEN 'INACTIVE' END; END;
Now it is literally impossible for this function to cause an ORA-06503 error. Either the last (and only) statement in the function is executed, or an exception is raised.
The PL/SQL compile time warnings feature can help you with this antipattern. There are no warnings for “Multiple RETURNS in your function.” But the PL/SQL compiler can tell you if it has identified at least one possible logic path in your function that would result in no RETURN being executed (and the raising of the ORA-06503 error at runtime): the PLW-05005 warning.
Assuming that the first version of status_desc with multiple returns is still compiled into the database, I will see the following when I turn on warnings and recompile:
ALTER SESSION SET plsql_warnings = 'ENABLE:5005' / ALTER FUNCTION status_desc COMPILE / PLW-05005: subprogram STATUS_DESC returns without value at line 15
But the function is valid, so even if the compiler produces this warning, the function can still be executed.
If that bothers you, tell the compiler to treat PLW-05005 as a compile error (because you really don’t want to embarrass yourself in the eyes of your users on this one):
ALTER SESSION SET plsql_warnings = 'ERROR:5005' / ALTER FUNCTION status_desc COMPILE / PLS-05005: subprogram STATUS_DESC returns without value at line 15
Note that the message now starts with “PLS” and not “PLW”. This indicates that 5005 was treated as an error and not a warning. The status of this function is now INVALID.
The situation with loops is similar to that with functions, but the thing to watch out for is one or more EXIT statements, rather than RETURNs. Consider the following block:
BEGIN FOR indx IN 1 .. my_collection.COUNT LOOP ... some code ... IF my_collection (indx) = l_termination_value THEN EXIT; END IF; END LOOP; END;
There are a couple of problems with this block. First of all, from the standpoint of code compactness, that IF statement should be replaced with an EXIT WHEN, as in
EXIT WHEN my_collection (indx) = l_termination_value
But the bigger problem is that the EXIT WHEN should not be used inside a FOR loop. A FOR loop says to anyone reading the code: “I am going to execute the body N times.” In other words, the built-in way “out” of a FOR loop is to execute the body the Nth time.
If, however, there is an EXIT statement inside the FOR loop, you have a potential second exit path. In this case, you should switch to either a simple loop or a WHILE loop. Here’s the WHILE loop implementation:
DECLARE c_coll_count PLS_INTEGER := my_collection.COUNT; l_found_terminator BOOLEAN := FALSE; l_index PLS_INTEGER := 1; BEGIN WHILE (NOT l_found_terminator AND l_index < c_coll_count) LOOP ... some code ... l_found_terminator := my_collection (indx) = l_termination_value; l_index := l_index + 1; END LOOP; END;
I am certain that some readers will object. This rewrite contains a lot more code. Surely the original version is clear enough, even if there are multiple exit pathways?
For this simple example, I would agree. But when you are working on a more complex piece of code and a loop body that contains 50 or 100 lines of code, the possible pain of debugging will better justify the rewrite.
When we build applications, we have to think of and code everything. As a result, even the smallest points of confusion or ambiguity can accumulate into a hard-to-manage application. Every resolution of an antipattern, either large or small, contributes to a high-quality application that developers can understand and enhance more easily.
Each PL/SQL article offers a quiz to test your knowledge of the information provided in it. The quiz appears below and also at PL/SQL Challenge (plsqlchallenge.com), a website that offers online quizzes on the PL/SQL language as well as SQL, Oracle Application Express, and database design.
Here is your quiz for this article:
Which of these choices display “-6502” after execution?
CREATE OR REPLACE PROCEDURE plch_bad_proc IS l_number NUMBER; BEGIN l_number := 'abc'; END; / BEGIN plch_bad_proc; /* Rest of program */ NULL; EXCEPTION WHEN VALUE_ERROR THEN DBMS_OUTPUT.put_line (SQLCODE); END; /
CREATE OR REPLACE FUNCTION plch_bad_func RETURN INTEGER IS l_number NUMBER; BEGIN l_number := 'abc'; RETURN 0; EXCEPTION WHEN OTHERS THEN RETURN SQLCODE; END; / DECLARE l_status PLS_INTEGER; BEGIN l_status := plch_bad_func; IF l_status = 0 THEN /* Continue! */ NULL; ELSE DBMS_OUTPUT.put_line (l_status); /* Rest of program */ NULL; END IF; END; /
CREATE OR REPLACE PACKAGE plch_err_pkg IS most_recent_error PLS_INTEGER := 0; END; / CREATE OR REPLACE PROCEDURE plch_bad_proc IS l_number NUMBER; BEGIN l_number := 'abc'; EXCEPTION WHEN OTHERS THEN plch_err_pkg.most_recent_error := SQLCODE; END; / BEGIN plch_bad_proc; IF plch_err_pkg.most_recent_error = 0 THEN /* Continue! */ NULL; ELSE DBMS_OUTPUT.put_line (plch_err_pkg.most_recent_error); plch_err_pkg.most_recent_error := 0; END IF; END; /
DOWNLOAD Oracle Database 12c
TEST your PL/SQL knowledge
READ more about PL/SQL
Photography by Dmitri Popov, Unsplash