G-4310: Never use GOTO statements in your code.

Major

Maintainability, Testability

Reason

Code containing gotos is hard to format. Indentation should be used to show logical structure and gotos have an effect on logical structure. Trying to use indentation to show the logical structure of a goto, however, is difficult or impossible.

Use of gotos is a matter of religion. In modern languages, you can easily replace nine out of ten gotos with equivalent structured constructs. In these simple cases, you should replace gotos out of habit. In the hard cases, you can break the code into smaller routines; use nested ifs; test and retest a status variable; or restructure a conditional. Eliminating the goto is harder in these cases, but it's good exercise.

Example (bad)

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
create or replace package body my_package is
   procedure password_check (in_password in varchar2) is 
      k_digitarray  constant string(10 char)   := '0123456789';
      k_lower_bound constant simple_integer := 1;
      k_errno       constant simple_integer := -20501;
      k_errmsg      constant string(100 char)  := 'Password must contain a digit.';
      l_isdigit     boolean     := false;
      l_password_length  pls_integer;
      l_array_length   pls_integer;
   begin
      l_password_length := length(in_password);
      l_array_length    := length(k_digitarray);

      <<check_digit>>
      for i in k_lower_bound .. l_array_length
      loop
         <<check_pw_char>>
         for j in k_lower_bound .. l_password_length
         loop
            if substr(in_password, j, 1) = substr(k_digitarray, i, 1) then
               l_isdigit := true;
               goto check_other_things;
            end if;
         end loop check_pw_char;
      end loop check_digit;

      <<check_other_things>>
      null;

      if not l_isdigit then
         raise_application_error(k_errno, k_errmsg);
      end if;
   end password_check;
end my_package;
/

Example (better)

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
create or replace package body my_package is
   procedure password_check (in_password in varchar2) is 
      k_digitarray  constant string(10 char)   := '0123456789';
      k_lower_bound constant simple_integer := 1;
      k_errno       constant simple_integer := -20501;
      k_errmsg      constant string(100 char)  := 'Password must contain a digit.';
      l_isdigit         boolean := false;
      l_password_length pls_integer;
      l_array_length    pls_integer;
   begin
      l_password_length := length(in_password);
      l_array_length    := length(k_digitarray);

      <<check_digit>>
      for i in k_lower_bound .. l_array_length
      loop
         <<check_pw_char>>
         for j in k_lower_bound .. l_password_length
         loop
            if substr(in_password, j, 1) = substr(k_digitarray, i, 1) then
               l_isdigit := true;
               exit check_digit; -- early exit condition
            end if;
         end loop check_pw_char;
      end loop check_digit;

      <<check_other_things>>
      null;

      if not l_isdigit then
         raise_application_error(k_errno, k_errmsg);
      end if;
   end password_check;
end my_package;
/

Example (good)

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
create or replace package body my_package is
   procedure password_check (in_password in varchar2) is
      k_digitpattern constant string(2 char)   := '\d';
      k_errno        constant simple_integer := -20501;
      k_errmsg       constant string(100 char)  := 'Password must contain a digit.';
   begin
      if not regexp_like(in_password, k_digitpattern) 
      then
         raise_application_error(k_errno, k_errmsg);
      end if;
   end password_check;
end my_package;
/