Tag: code-snippet-of-the-day

Code I Regret: Refactoring as Penance

Recently I refactored some PL/SQL for sending emails – code that I wrote way back in 2004. The number of “WTF“‘s per minute has not been too high; however, I’ve cringed more times than I’d like…

1. Overly-generic parameter types

When you send an email, it will have at least one recipient, and it may have many recipients. However, no email will have more than one sender. Yet, I wrote the package procedure like this:

TYPE address_type IS RECORD
 (name          VARCHAR2(100)
 ,email_address VARCHAR2(200)
TYPE address_list_type IS TABLE OF address_type
 (i_sender     IN address_list_type
 ,i_recipients IN address_list_type
 ,i_subject    IN VARCHAR2
 ,i_message    IN VARCHAR2

Why I didn’t have i_sender be a simple address_type, I can’t remember. Internally, the procedure only looks at i_sender(1) – if a caller were to pass in a table of more than one sender, it raises an exception.

2. Functional programming to avoid local variables

Simple is best, and there’s nothing wrong with using local variables. I wish I’d realised these facts when I wrote functions like this:

FUNCTION address
 (i_name          IN VARCHAR2
 ,i_email_address IN VARCHAR2
 ) RETURN address_list_type;
FUNCTION address
 (i_address       IN address_list_type
 ,i_name          IN VARCHAR2
 ,i_email_address IN VARCHAR2
 ) RETURN address_list_type;

All that so that callers can avoid *one local variable*:

 (i_sender     => EMAIL_PKG.address('joe','joe@company.com')
 ,i_recipients => EMAIL_PKG.address(
                  'jill', 'jill@company.com')
                 ,'bob', 'bob@company.com')
 ,i_subject    => 'hello'
 ,i_message    => 'world'

See what I did there with the recipients? Populating an array on the fly with just function calls. Smart eh? But rather useless, as it turns out; when we need to send multiple recipients, it’s usually populated within a loop of unknown sized, so this method doesn’t work anyway.

Go ahead – face your past and dig up some code you wrote 5 years ago or more. I think, if you don’t go “WTF!” every now and then, you probably haven’t learned anything or improved yourself in the intervening years. Just saying :)

Negative, Captain

Seen in the wild:

... WHERE substr(amount,0, 1) != '-'

If you wanted to query a table of monetary transactions for any refunds (i.e. where the transaction amount is negative), how would you do it? Perhaps you’d think about avoiding problems that might occur if the default number format were to change, hm?

(before you say it: no, there is no index on amount, so it wasn’t a misguided attempt to avoid an index access path…)

Generic Audit

Looking at the column comments on this table, I can sympathise with the poor soul who after painstakingly describing 100 columns just decided that it just wasn’t worth it…

table audit_history (column comments in quotes):

id number(18) “Unique identifier”
aud_table_name varchar2(30) “audit record table”
audit_action varchar2(50) “audit action e.g. update”
aud_timestamp date “timestamp of the audit”
aud_user_id varchar2(100) “user performing the action”
aud_col_1 varchar2(4000) “audit column 1”
aud_col_2 varchar2(4000) “audit column 2”
aud_col_3 varchar2(4000) “audit column 3”
… (etc.) …
aud_col_99 varchar2(4000) “audit column 99”
aud_col_100 varchar2(4000) “audit column 100”
aud_col_101 varchar2(4000)
aud_col_102 varchar2(4000)
… (etc.) …
aud_col_139 varchar2(4000)
aud_col_140 varchar2(4000)

Tip: Don’t let anyone even think about using this kind of design for their change history auditing requirements. It might look elegant to them, but it is not good. Just, don’t.