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. Using indentation to show the logical structure of a goto and its target, however, is difficult or impossible. (...)
Use of gotos is a matter of religion. My dogma is that in modern languages, you can easily replace nine out of ten gotos with equivalent sequential constructs. In these simple cases, you should replace gotos out of habit. In the hard cases, you can still exorcise the goto in nine out of ten cases: You can break the code into smaller routines, use try-finally, 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 (...).
-- McConnell, Steve C. (2004). Code Complete. Second Edition. Microsoft Press.
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
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)
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
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)
1
2
3
4
5
6
7
8
9
10
11
12 | 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;
/
|