Wednesday, August 19, 2009

PL/SQL: Coding Practices

I've struggled this week to write. Lately I've had lots of technical content and not much philosophical content. I have lots of philosophical content now, but the blog isn't the place for it...at least not now.

The big project I'm working on now is refactoring our payment processing system. We interface with multiple gateways for redundancy purposes. The code is comprised of 3 stand-alone procedures. One of those procedures has (had) 17 private procedures/functions in it. On one hand, it made sense, since everything was a stand-alone procedure or function, you didn't want all these dependencies on other objects. On the other hand, testing was virtually impossible. If you needed to make a change to private procedure #13, you had to run through an almost infinite number of test cases to ensure that you covered that particular case.

I've now moved those 3 procedures into a package. Theoretically, it's hot deployable now as long as we don't change the package signature. That's a big win in my book. I've also moved those 17 private functions into their own individual procedures and functions that can be exposed via the package specification for unit testing purposes (in production they will be private as nothing else needs access). The hardest part of that effort was there were no parameters being passed to the procedure, it just relied on the declared variables. So for each and every one of those I had to figure out what it relied on to work and what variables it set. No small task.

Here is the table:
CREATE TABLE t
(
id NUMBER PRIMARY KEY,
col_1 NUMBER(1) DEFAULT 0 NOT NULL,
col_2 NUMBER(1) DEFAULT 0 NOT NULL,
col_3 NUMBER(1) DEFAULT 0 NOT NULL,
start_date DATE DEFAULT SYSDATE NOT NULL,
end_date DATE
);
and a procedure (which does nothing obviously)
CREATE OR REPLACE
PROCEDURE update_t
( p_id IN NUMBER,
p_col_1 IN INTEGER DEFAULT 0,
p_col_2 IN INTEGER DEFAULT 0,
p_col_3 IN INTEGER DEFAULT 0 )
IS
BEGIN
NULL;
END update_t;
What's the best way to integrate the procedure into your code? I've seen this:
CREATE OR REPLACE
PROCEDURE some_other_procedure
( p_id NUMBER,
p_variable VARCHAR2(1) )
IS
BEGIN
IF p_variable = 'A' THEN
update_t
( p_id => p_id,
p_col_3 => 1 );
ELSIF p_variable = 'B' THEN
update_t
( p_id => p_id,
p_col_2 => 1,
p_col_3 => 1 );
ELSIF p_variable = 'C' THEN
update_t
( p_id => p_id,
p_col_1 => 1 );
ELSIF p_variable = 'D' THEN
update_t
( p_id => p_id,
p_col_1 => 1,
p_col_3 => 1 );
END IF;
END some_other_procedures;
Since I default the input parameters to 0, I didn't have to specify each individual parameter every time I called it. I like that.

I don't much like having 4 separate calls to UPATE_T though.

1. It makes it difficult (without further logging), to determine where exactly it's being called in the control statement.
2. Seems like a waste of space. P_ID is always going to be the same, why set it 4 times?

I decided to make just one call to UPDATE_T. I create local variables, then set them in the control statement, and then make the call to UPDATE_T.
CREATE OR REPLACE
PROCEDURE some_other_procedure
( p_id NUMBER,
p_variable VARCHAR2(1) )
IS
l_col_1 INTEGER := 0;
l_col_2 INTEGER := 0;
l_col_3 INTEGER := 0;
BEGIN
IF p_variable = 'A' THEN
l_col_3 := 1;
ELSIF p_variable = 'B' THEN
l_col_2 := 1;
l_col_3 := 1;
ELSIF p_variable = 'C' THEN
l_col_1 := 1;
ELSIF p_variable = 'D' THEN
l_col_1 := 1;
l_col_3 := 1;
END IF;

update_t
( p_id => p_id,
p_col_1 => l_col_1,
p_col_2 => l_col_2,
p_col_3 => l_col_3 );

END some_other_procedures;
Not much savings in space (and sometimes you'll actually have more), but for me, this is much easier to read. If I have to debug this, it feels a lot easier to concentrate on the control statement without the calls to UPDATE_T.

What do you do in these kinds of situations? Same as me? Different? Think I'm off my rocker (yeah, I know some of you do)?

2 comments:

  1. You could also use case expressions. They support a functional programming style, which often makes the data flow clearer than using side effects (variable assignments in a big if).

    ReplyDelete
  2. Funny you mention that because in the real code that's exactly what I did. There were 3 or 4 ELSIF all on P_ID (in the example posted) so it lent itself nicely to the CASE statement.

    I like it better most times as I find it easier to read.

    ReplyDelete