Code re-use in PL/SQL might just need SQL

Posted by

I was doing some code review today of a PL/SQL package, and came across a routine to generate a list of employees for a department.  Truth be told, it wasn’t employees and departments but some anonymity is called for here. Smile


SQL> create or replace
  2  function emp_list(p_dept varchar2) return varchar2 is
  3    l_emps varchar2(4000) := ':';
  4  begin
  5    for i in (
  6      select empno
  7      from   emp
  8      where  deptno = p_dept  )
  9    loop
 10      l_emps := l_emps || i.empno ||':';
 11    end loop;
 12    return l_emps;
 13  end;
 14  /

Function created.

SQL>
SQL> select emp_list(30) from dual;

EMP_LIST(30)
----------------------------------------------------------------
:7499:7521:7654:7698:7844:7900:

Savvy readers will straight away know that rather than “reinvent the wheel”, the function could re-coded with a LISTAGG function, and since at that point, it is just a standard SQL expression, then potentially the PL/SQL function is not required at all. (We’d only know that by knowing the full context of the function’s usage).


SQL> create or replace
  2  function emp_list(p_dept varchar2) return varchar2 is
  3    l_emps varchar2(4000);
  4  begin
  5    select ':'||listagg(empno,':') within group ( order by empno )||':'
  6    into l_emps
  7    from   emp
  8    where  deptno = p_dept;
  9    return l_emps;
 10  end;
 11  /

Function created.

SQL>
SQL> select emp_list(30) from dual;

EMP_LIST(30)
---------------------------------------------------------------------------
:7499:7521:7654:7698:7844:7900:

However, we can’t be too critical here. We don’t know the background and history of the code – perhaps it was written well before LISTAGG was available? So whilst its an obvious improvement, I’m happy to let that one slide.

But it is easy to be led astray by the allure of “reuse”, and the very next procedure in the package implements the business requirement of: “Does a nominated employee belong to a nominated department?”

Let’s look at the implementation.


SQL> create or replace
  2  function emp_in_dept(p_emp int, p_dept int) return boolean is
  3    l_emps varchar2(4000);
  4  begin
  5    l_emps := emp_list(p_dept);            <<<==== our previous function
  6    return instr(l_emps,':'||p_emp||':') > 0;
  7  end;
  8  /

Function created.

SQL>
SQL> set serverout on
SQL> begin
  2    --
  3    -- should be true
  4    --
  5    if emp_in_dept(7900,30) then dbms_output.put_line('Yes 7900,30'); end if;
  6    --
  7    -- should be false
  8    --
  9    if emp_in_dept(7499,20) then dbms_output.put_line('Yes 7499,20'); end if;
 10  end;
 11  /
Yes 7900,30

PL/SQL procedure successfully completed.

All I can really say is …. don’t do that. There really is no excuse for this one, because even without the LISTAGG version, the first procedure indicates that:

  • the developer knew of the EMP table and its columns
  • the developer knew the data was the DEPT/EMP relationship
  • the developer knew how to query the table for a given DEPT

so why not implement the routine with the trivial SQL to meet the requirement?


SQL> create or replace
  2  function emp_in_dept(p_emp int, p_dept int) return boolean is
  3    l_dept int;
  4  begin
  5    select count(case when deptno = p_dept then 1 end)
  6    into   l_dept
  7    from   emp
  8    where  empno = p_emp;
  9    return l_dept > 0;
 10  end;
 11  /

Function created.

Therein lies the rub. As developers, we have drilled into us constantly “reuse, reuse, reuse” as if re-using code is the goal as opposed a good guiding principle.

So please…by all means look for opportunities to modularise and re-use your code, but don’t force it into your codebase just for sake for claiming your doing it. That isn’t code RE-use, that is brain NO-use Smile

Got some thoughts? Leave a comment

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google photo

You are commenting using your Google account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s

This site uses Akismet to reduce spam. Learn how your comment data is processed.