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 mental exercise.

Excerpt of Using gotos by Steven C. McConnell.

Example (bad)

CREATE OR REPLACE PACKAGE BODY my_package IS
   PROCEDURE password_check (in_password IN VARCHAR2) IS 
      co_digitarray  CONSTANT STRING(10 CHAR)   := '0123456789';
      co_lower_bound CONSTANT SIMPLE_INTEGER := 1;
      co_errno       CONSTANT SIMPLE_INTEGER := -20501;
      co_errmsg      CONSTANT STRING(100 CHAR)  := 'Password must contain a digit.';
      l_isdigit      BOOLEAN     := FALSE;
      l_len_pw       PLS_INTEGER;
      l_len_array    PLS_INTEGER;
   BEGIN
      l_len_pw    := LENGTH(in_password);
      l_len_array := LENGTH(co_digitarray);

      <<check_digit>>
      FOR i IN co_lower_bound .. l_len_array
      LOOP
         <<check_pw_char>>
         FOR j IN co_lower_bound .. l_len_pw
         LOOP
            IF SUBSTR(in_password, j, 1) = SUBSTR(co_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(co_errno, co_errmsg);
      END IF;
   END password_check;
END my_package;
/

Example (better)

CREATE OR REPLACE PACKAGE BODY my_package IS
   PROCEDURE password_check (in_password IN VARCHAR2) IS 
      co_digitarray  CONSTANT STRING(10 CHAR)   := '0123456789';
      co_lower_bound CONSTANT SIMPLE_INTEGER := 1;
      co_errno       CONSTANT SIMPLE_INTEGER := -20501;
      co_errmsg      CONSTANT STRING(100 CHAR)  := 'Password must contain a digit.';
      l_isdigit      BOOLEAN     := FALSE;
      l_len_pw       PLS_INTEGER;
      l_len_array    PLS_INTEGER;
   BEGIN
      l_len_pw    := LENGTH(in_password);
      l_len_array := LENGTH(co_digitarray);

      <<check_digit>>
      FOR i IN co_lower_bound .. l_len_array
      LOOP
         <<check_pw_char>>
         FOR j IN co_lower_bound .. l_len_pw
         LOOP
            IF SUBSTR(in_password, j, 1) = SUBSTR(co_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(co_errno, co_errmsg);
      END IF;
   END password_check;
END my_package;
/

Example (good)

CREATE OR REPLACE PACKAGE BODY my_package IS
   PROCEDURE password_check (in_password IN VARCHAR2) IS
      co_digitpattern CONSTANT STRING(10 CHAR)   := '\d';
      co_errno        CONSTANT SIMPLE_INTEGER := -20501;
      co_errmsg       CONSTANT STRING(100 CHAR)  := 'Password must contain a digit.';
   BEGIN
      IF NOT REGEXP_LIKE(in_password, co_digitpattern) 
      THEN
         raise_application_error(co_errno, co_errmsg);
      END IF;
   END password_check;
END my_package;
/