G-7130: Always use parameters or pull in definitions rather than referencing external variables in a local program unit.

Major

Maintainability, Reliability, Testability

Reason

Local procedures and functions offer an excellent way to avoid code redundancy and make your code more readable (and thus more maintainable). Your local program refers, however, an external data structure, i.e., a variable that is declared outside of the local program. Thus, it is acting as a global variable inside the program.

This external dependency is hidden, and may cause problems in the future. You should instead add a parameter to the parameter list of this program and pass the value through the list. This technique makes your program more reusable and avoids scoping problems, i.e. the program unit is less tied to particular variables in the program. In addition, unit encapsulation makes maintenance a lot easier and cheaper.

Example (bad)

create or replace package body employee_api is
   procedure calc_salary(in_employee_id in integer) is
      co_employee_id constant employees.employee_id%type := in_employee_id;
      r_emp          employees%rowtype;

      function commission return number is -- NOSONAR: non-deterministic
         l_commission employees.salary%type := 0;
      begin
         if r_emp.commission_pct is not null then
            l_commission := r_emp.salary * r_emp.commission_pct;
         end if;

         return l_commission;
      end commission;
   begin
      select *
        into r_emp
        from employees
       where employee_id = co_employee_id;

      sys.dbms_output.put_line(r_emp.salary + commission());
   exception
      when no_data_found then
         null;
      when too_many_rows then
         null;
   end calc_salary;
end employee_api;
/

Example (good)

create or replace package body employee_api is
   procedure calc_salary(in_employee_id in integer) is
      co_employee_id constant employees.employee_id%type := in_employee_id;
      r_emp          employees%rowtype;

      function commission(
         in_salary   in number
        ,in_comm_pct in number
      )
         return number
         deterministic
      is
         co_salary    constant employees.salary%type         := in_salary;
         co_comm_pct  constant employees.commission_pct%type := in_comm_pct;
         l_commission employees.salary%type                  := 0;
      begin
         if in_comm_pct is not null then
            l_commission := co_salary * co_comm_pct;
         end if;

         return l_commission;
      end commission;
   begin
      select *
        into r_emp
        from employees
       where employee_id = co_employee_id;

      sys.dbms_output.put_line(
         r_emp.salary + commission(in_salary   => r_emp.salary
                                  ,in_comm_pct => r_emp.commission_pct)
      );
   exception
      when no_data_found then
         null;
      when too_many_rows then
         null;
   end calc_salary;
end employee_api;
/