```rpg Bad theme={null} D VAR2 S 5 0 IMPORT * Noncompliant C MOVE MYVAR VAR2 C/FREE // Noncompliant VAR2=MYVAR; /END-FREE ``` ```rpg Fix theme={null} ```

The DEBUG(\*YES) and DUMP statements are useful during development and debugging, but could expose sensitive information to attackers and should not be included in production code.

```rpg Bad theme={null} H*------------------------------------------------------------------------- H DATEDIT(*YMD) DEBUG(*YES) H************************************************************************** C SR990 BegSR C 'CVTERR' DUMP DUMP for error C Move *on *INLR ``` ```rpg Fix theme={null} H*------------------------------------------------------------------------- H DATEDIT(*YMD) H************************************************************************** C SR990 BegSR C Move *on *INLR ```

Even if there’s no data to be returned from a procedure, each procedure should at least return a boolean to indicate when procedure execution failed, so callers can respond accordingly.

```rpg Bad theme={null} D MyProc PI D param1 10A const /free // do work... return; // Noncompliant /end-free ``` ```rpg Fix theme={null} D MyProc PI N D param1 10A const /free // do work... return *ON; /end-free ```

While \`END will adequately close a statement, it is less clear than the use of the relevant, statement-specific ENDxx. Thus, the statement-specific version is preferred to facilitate code maintenance and enhance clarity.

This rule is applied to the following operations: CASxx, DO, DOU, DOUxx, DOW, DOWxx, FOR, IF, IFxx and SELECT\` groups.

```rpg Bad theme={null} C W0PKEX DOUEQ W0ON ... C END C W0PKEX IFEQ W0ON ... C END ``` ```rpg Fix theme={null} /free if x = 1; ... end; /end-free ```

The complexity of an expression is defined by the number of AND, OR.

A single expression’s complexity should not become too high to keep the code readable.

```rpg Bad theme={null} C IF (condition1 AND condition2) C OR (condition2 AND condition2) C OR (condition3 AND condition4) ``` ```rpg Fix theme={null} /free IF A=X OR A=Y AND A=Z AND B=Y OR B=Z; ... ENDIF; /end-free ```

When multiple, adjacent MONITOR statements have duplicate ON-ERROR blocks, they should be merged to consolidate the ON-ERROR logic for cleaner, more readable code. Note that this applies even when there is intervening code outside any MONITOR block.

```rpg Bad theme={null} /free monitor; // do something... on-error; CALLP HandleError(*param); endmon; // do un-monitored thing monitor; // do something else... on-error; // Noncompliant CALLP HandleError(*param); endmon; /end-free ``` ```rpg Fix theme={null} /free monitor; // do something... // do un-monitored thing // do something else... on-error; CALLP HandleError(*param); endmon; ```

Shared coding conventions allow teams to collaborate effectively. This rule checks that labels are on lines by themselves, which makes them more immediately visible as such.

```rpg Bad theme={null} LBLVAL: IF COND(&ERRFLG = ' ') THEN (DO) CHGVAR VAR(&MSG) VALUE('Validation Completed') ENDDO ``` ```rpg Fix theme={null} LBLVAL: IF COND(&ERRFLG = ' ') THEN (DO) CHGVAR VAR(&MSG) VALUE('Validation Completed') ENDDO ```

The format used to write a record to a file should be cleared before each use. Otherwise stale data left in the format from previous records may be saved into the current record if it does not have data for all the fields in the format.

```rpg Bad theme={null} C IF X <> Y ... C ENDIF C WRITE RECFMT ``` ```rpg Fix theme={null} /free if x <> y; ... endif; write recfmt; /end-free ```

Leaving an ON-ERROR block empty means that the exception in question is neither handled nor passed forward to callers for handling at a higher level. Suppressing errors rather than handling them could lead to unpredictable system behavior and should be avoided.

```rpg Bad theme={null} /free monitor; // ... on-error *FILE; // Noncompliant endmon; /end-free ``` ```rpg Fix theme={null} /free monitor; // ... on-error *FILE; handleFileError(); endmon; /end-free ```

The use of UNDERLINE makes normal text printed to the screen difficult to read, and prevents titles from being read at all. For these reasons, UNDERLINE should not be used.

```rpg Bad theme={null} R DETAIL1 * Noncompliant; next line contains UNDERLINE NAME 25 2 2UNDERLINE ADD1 25 3 2 R DETAIL3 ADD2 25 2SPACEB(1) ``` ```rpg Fix theme={null} R DETAIL1 NAME 25 2 2 ADD1 25 3 2 R DETAIL3 ADD2 25 2SPACEB(1) ```

Using a unique symbolic \`PLIST name provides a single definition point for the parameters used by any external routine being called. This ensures that the same information is passed every time the external routine is called, reducing the potential for mistakes in operation.

This rule flags non-compliant instances of CALL and CALLB\`.

```rpg Bad theme={null} C CALLB(D) 'XR01' C PARM WSXR07 C PARM WSFLD1 ``` ```rpg Fix theme={null} C P0XR07 PLIST C PARM WSXR07 C PARM WSFLD1 C CALLB 'XR07' P0XR07 ```

Each program should contain an initial section of H\* comments lines, which includes a copyright and/or license statement. This rule must be fed with the header text that is expected in the header.

```rpg Bad theme={null} H* Copyright (C) 2008-2013 SonarSource H* mailto:contact AT sonarsource DOT com H* H* SonarQube is free software; you can redistribute it and/or H* modify it under the terms of the GNU Lesser General Public H* License as published by the Free Software Foundation; either H* version 3 of the License, or (at your option) any later version. H* H* SonarQube is distributed in the hope that it will be useful, H* but WITHOUT ANY WARRANTY; without even the implied warranty of H* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU H* Lesser General Public License for more details. H* H* You should have received a copy of the GNU Lesser General Public License H* along with this program; if not, write to the Free Software Foundation, H* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. ``` ```rpg Fix theme={null} ```

Historically /COPY procedures contained all of the specifications required for each procedure, so for example xxxxF /COPY would be used to copy the F lines from another F spec. However, this practice results in code that is difficult to understand and maintain. Instead, the use of /COPY statements should be replaced with explicit declarations.

```rpg Bad theme={null} F/COPY HRSILERPG,RSX36 ``` ```rpg Fix theme={null} FSSINRTP IF E K DISK INFSR(*PSSR) F* F* Interest Rates File F* ```

Turning on the \`\*srcstmt header option means that any line numbers cited in error statements will actually correspond correctly to the code. Otherwise, object line numbers will be shown, making errors difficult to debug.

The \*srcstmt option is also recommended for accurate line numbers during debugging, particularly in combination with \*nodebugio, (H option(\*srcstmt:\*nodebugio)\`) which prevents debug operations from stopping on I/O.

```rpg Bad theme={null} H* H* AMENDED BY : G. Ann Campbell H* DATE : 16JUL2014 H* H24 - Miscellaneous Online Maintenance/Enquiry/Printing H* F/EJECT ``` ```rpg Fix theme={null} H* H* AMENDED BY : G. Ann Campbell H* DATE : 16JUL2014 H* H24 - Miscellaneous Online Maintenance/Enquiry/Printing H* H option(*srcstmt) F/EJECT ```

If a file is defined without the `USROPN statement then the natural RPG logic will deal with opening and closing it. However, files defined with USROPN, must be both explicitly OPENed and CLOSE`d.

```rpg Bad theme={null} FEWPCCR1 O E PRINTER USROPN F INFDS(WSFD01) F INFSR(*PSSR) C OPEN EWPCCR1 C CLOSE *ALL ``` ```rpg Fix theme={null} FEWPCCR1 O E PRINTER USROPN F INFDS(WSFD01) F INFSR(*PSSR) /free open EWPCCR1; close *ALL; /end-free ```

Using both SPACEA (space-after) and SPACEB (space-before) in the same printer file can make it difficult to understand how many blank lines will be printed where. Therefore either one command or the other should be used but not both.

```rpg Bad theme={null} A R REP001 TEXT('Detail line. ') A SPACEB(1) A SPACEA(1) A 2'Additional Details for Student :' A VAR01 6 36TEXT('Additional Details for + A Student') ``` ```rpg Fix theme={null} A R REP001 TEXT('Detail line. ') A SPACEB(1) A 2'Additional Details for Student :' A VAR01 6 36TEXT('Additional Details for + A Student') A SPACEB(1) ```

A Select or a chain of If/Elseif statements is evaluated from top to bottom. At most, only one branch will be executed: the first one with a condition that evaluates to true.

Therefore, duplicating a condition automatically leads to dead code. Usually, this is due to a copy/paste error. At best, it’s simply dead code and at worst, it’s a bug that is likely to induce further bugs as the code is maintained, and obviously it could lead to unexpected behavior.

```rpg Bad theme={null} C IF X = 1 C EXSR SR01 C ELSEIF X = 1 Noncompliant C EXSR SR02 C ENDIF ``` ```rpg Fix theme={null} /free IF param = 1; doX(); ELSEIF param = 2; doY(); ELSEIF param = 1; // Noncompliant doZ(); ENDIF; SELECT; WHEN param = 1; doX(); WHEN param = 2; doY(); WHEN param = 1; // Noncompliant doZ(); ENDSL; /end-free ```

Using a named constant to refer to an indicator makes the content of the field clearer, and therefore makes the code easier to read and maintain.

```rpg Bad theme={null} D accountTotal c 25 /Free If *In(accountTotal); // Process contents... EndIf; ``` ```rpg Fix theme={null} ```

Having all the pieces in front of you makes it easier to understand and debug or maintain a piece of code. Unfortunately, that’s often not the case with compile-time arrays, since they are often separated from the code that uses them by many, many lines. Instead, data structures are often a better option.

```rpg Bad theme={null} D directions S 5 0 dim(4) ctdata perrcd(1) ``` ```rpg Fix theme={null} D dirData DS D 5 Inz('North') D 5 Inz('South') D 5 Inz('East') D 5 Inz('West') D directions 5 Dim(4) Overlay(dirData) ```

Externally described files standardize file access, and result in simpler, easier to read code.

```rpg Bad theme={null} FEPBLMSL2 IF F 100 DISK INFSR(*PSSR) F* F* ``` ```rpg Fix theme={null} F* FIPBDCCP IF E K DISK INFSR(*PSSR) F* FEP210 CF E WORKSTN INFSR(*PSSR) F SFILE(EP21003S:RELN03) FEP471R1 O E PRINTER INFSR(*PSSR) F USROPN F INFDS(W1SF01) ```

Testing %PARMS before the use of one or more parameters is a clear indication in the code that at least some parameters are treated as optional. However, failure to actually mark those parameters as such with options( \*nopass ) denies callers the ability to treat them as optional. Since all parameters after the first one explicitly declared options( \*nopass ) are also implicitly optional, this marking is only required once. However, for clarity you should consider explicitly marking all optional parameters.

```rpg Bad theme={null} DCL-PR VIOLATION1 EXTPGM; // Noncompliant LASTNAME Char(10); FIRSTNAME Char(10); END-PR; DCL-PI VIOLATION1; LASTNAME Char(10); FIRSTNAME Char(10); END-PI; dsply FIRSTNAME; IF %PARMS > 1; dsply LASTNAME; EndIf; *inlr = *on; ``` ```rpg Fix theme={null} DCL-PR VIOLATION1 EXTPGM; LASTNAME Char(10); FIRSTNAME Char(10) options( *nopass ); END-PR; DCL-PI VIOLATION1; LASTNAME Char(10); FIRSTNAME Char(10) options( *nopass ); END-PI; dsply FIRSTNAME; IF %PARMS > 1; dsply LASTNAME; EndIf; *inlr = *on; ```

Eval and Callp are the only two exceptions to the rule that each free-format line must start with an operation code. Since you can leave these two opcodes out, you should for cleaner, more readable code.

```rpg Bad theme={null} Eval Regpay = hours * perHour; Callp calcTax(); ``` ```rpg Fix theme={null} Regpay = hours * perHour; calcTax(); ```

This rule applies whenever an \`IF statement is followed by one or more ELSEIF statements; the final ELSEIF should be followed by an ELSE statement.

The requirement for a final ELSE statement is defensive programming.

The ELSE statement should either take appropriate action or contain a suitable comment as to why no action is taken. This is consistent with the requirement to have a final OTHER clause in a SELECT\` statement.

```rpg Bad theme={null} C IF RESULT = 0 C ... C ELSEIF RESULT > 0 C ... C ENDIF ``` ```rpg Fix theme={null} /free if result = 0; ... elseif result > 0; ... endif; /end-free ```

Using a unique \`KLIST to access files ensures that the correct key is used at all times and creates a standard method of accessing the data throughout the code, simplifying maintenance and improving readability.

This rule flags non-compliant instances of CHAIN, DELETE, READE, READPE, SETGT, and SETLL\` operations.

```rpg Bad theme={null} C 'D001 B' CHAIN KYCUSTN 20 ... C DEPT CHAIN KYCUSTN 20 ``` ```rpg Fix theme={null} C DEPTKY KLIST C KFLD DEPT C KFLD SHIFT C DEPTKY CHAIN KYCUSTN 20 ```

Shared naming conventions allow teams to collaborate efficiently.

This rule raises an issue when a subroutine name does not match a provided regular expression.

For example, with the default provided regular expression ^SR\[a-zA-Z0-9]\*\$, the following subroutines:

```rpg Bad theme={null} C XXXXX BEGSR ... C ENDSR ``` ```rpg Fix theme={null} /free begsr yyyy; endsr; /end-free{code} ```

Nested control flow statements CASxx, DO, DOU, DOW, DOUxx, DOWxx, FOR, IF, IFxx, MONITOR and SELECT are often key ingredients in creating what’s known as "Spaghetti code". This code smell can make your program difficult to understand and maintain.

When numerous control structures are placed inside one another, the code becomes a tangled, complex web. This significantly reduces the code’s readability and maintainability, and it also complicates the testing process.

```rpg Bad theme={null} IF A <> B IF B <> C DOW B <> A IF A <> D ... ENDIF ENDDO ENDIF ENDIF ``` ```rpg Fix theme={null} if a <> b; if b <> c; dow b <> a; if a <> d; ... endif; enddo; endif; endif; ```

When storing numeric fields, using an odd number of digits allows the sign to be included in the storage area without wasting any space.

For example the number -1234567, when packed as 7 digits is stored as follows.

1357
        246-

thus taking up only 4 data slots. but when packed as 8 characters it uses 5 data slots

02467
        0135-

or when 7 digits, non-packed, it will be stored as 8 data slots, including the sign.

FFFFFFF6
        12345670

```rpg Bad theme={null} D NUMFLD S 8P 0 D NUMFLD S 7S 0 ``` ```rpg Fix theme={null} D NUMFLD S 7P 0 ```

Subprocedures and subroutines are both mechanisms to segregate logic, but subprocedures are preferred for three reasons:

their local files and variables make maintenance faster and cleaner. They allow you to create variables without worrying about name clashes, and to change fields without worrying about negatively impacting other parts of the program.
their local files and variables make code reuse easy.
they can be called with parameters as functions, yielding clearer more readable code.

```rpg Bad theme={null} D FirstName S 20A D Initial S 1A D LastName S 20A D FullName S 43A /free FirstName = 'John'; Initial = 'A'; LastName = 'Smith'; EXSR SPFullName; DSPLY FullName; ... begsr SPFullName; FullName = FirstName + ' ' + Initial + ' ' + LastName; endsr; /end-free ``` ```rpg Fix theme={null} /free DSPLY FullName('John':'A':'Smith'); ... /end-free P FullName B D FullName PI 43A D FirstName 20A Const D Initial 1A Const D LastName 20A Const /free return FirstName + ' ' + Initial + ' ' + LastName; /end-Free P E ```

Shared coding conventions allow teams to collaborate effectively. While it is possible to omit the specification letter before a /COPY statement, it is advisable not to do so because including a specification on the line indicates to other developers what type of statements will be added to the program by the copybook. Further, using a specification letter on the line makes it clearer to those who might be skimming the file that the line is not just a comment, but does contain code.

```rpg Bad theme={null} /COPY MBR1 ``` ```rpg Fix theme={null} I/COPY MBR1 ```

Every subroutine should be commented to explain its goal and how it works. This non-empty comment must be located before or after the subroutine definition.

```rpg Bad theme={null} C SR012 BEGSR C ... C ENDSR ``` ```rpg Fix theme={null} /free begsr SR012; ... endsr; /end-free ```

Global variables can seem like a handy way to pass state information around in a program, but the use of global variables only works well in very small programs. As the code base grows, you’ll need to understand every subprocedure’s impact on the global state in order to understand how the program works. This is a task that quickly becomes impossible.

To control the situation, only the main procedure should be allowed access to global variables; it can then pass that state information to subprocedures as parameters.

```rpg Bad theme={null} D FirstName S 20A D LastName S 20A /free FirstName = 'John'; LastName = 'Smith'; DSPLY FullName(); /end-free P FullName B D FullName PI 41A /free return FirstName + ' ' + LastName; /end-Free P E ``` ```rpg Fix theme={null} /free DSPLY FullName('John':'Smith'); /end-free P FullName B D FullName PI 41A D FirstName 20A Const D LastName 20A Const /free return FirstName + ' ' + LastName; /end-Free P E ```

Error handling should be defined on file operations to ensure correct processing of program failures rather than defaulting to an abnormal program end. This rule checks two things:

INFSR error handling is defined on file operations.
error handling is delegated to an appropriately named routine.

```rpg Bad theme={null} F* Noncompliant FIPBDCCP IF E K DISK FEP210 CF E WORKSTN F SFILE(EP21003S:RELN03) FEP471R1 O E PRINTER F USROPN F INFDS(W1SF01) ``` ```rpg Fix theme={null} FIPBDCCP IF E K DISK INFSR(*PSSR) FEP210 CF E WORKSTN INFSR(*PSSR) F SFILE(EP21003S:RELN03) FEP471R1 O E PRINTER INFSR(*PSSR) F USROPN F INFDS(W1SF01) ```

GOTO is an unstructured control flow statement. It makes code less readable and maintainable, and should only be used to branch to defined ENDSR points within the source. For normal loop functions use ITER or LEAVE instead.

```rpg Bad theme={null} C LOOP1 TAG ... C GOTO LOOP1 ``` ```rpg Fix theme={null} ```

Both the name and the field definitions of a parameter 2 data area used in an IN should be coded in the D specification. This ensures that all definition data is kept together, and the specific data is not manually extracted in the code, thus making the code easier to read and maintain.

```rpg Bad theme={null} D WSFD08 S 30A 0 C IN WSFD08 C MOVEL WSFD08 VAR9S ``` ```rpg Fix theme={null} D WSFD08 S 30A 0 /free in WSFD08; /end-free ```

The use of positional notation to describe the overlapping of one field on another can quickly become confusing, particularly in large, or complicated data structures. Instead, the OVERLAY keyword should be used to make such overlaps more readily identifiable.

```rpg Bad theme={null} D Message DS QUALIFIED D Id 7 * Noncompliant D Type 1 3 ``` ```rpg Fix theme={null} D Message DS QUALIFIED D Id 7 D Type 3 OVERLAY(Id) ```

Free-format syntax is preferred because it is clearer, easier to write, and easier for newcomers to the language than fixed-format syntax.

```rpg Bad theme={null} C IF A > 10 C EVAL B = 1 C ENDIF ``` ```rpg Fix theme={null} /free if A > 10; B = 1; endif; /end-free ```

There’s no need to repeat the program name in the free-format definition of an external program.

```rpg Bad theme={null} DCL-PR PGM001 EXTPGM('PGM001'); END-PR; ``` ```rpg Fix theme={null} DCL-PR PGM001 EXTPGM ; END-PR; ```

Because library names could change from environment to environment, they should never be hard-coded in a program. Instead, you should use a variable to specify the library name and set that variable with data external to the program.

```rpg Bad theme={null} CLRPFM FILE(XPJLIB/FILE01) ``` ```rpg Fix theme={null} CHGVAR VAR(&LIB01) VALUE(&PREFIX *CAT 'LIB') CLRPFM FILE(&LIB01/FILE01) ```

The SELECT statement should be used only to clearly define some new branches in the control flow. As soon as a WHEN clause contains too many statements this highly decreases the readability of the overall control flow statement. In such case, the content of WHEN clause should be extracted in a dedicated subroutine.

```rpg Bad theme={null} C SELECT C WHEN X=1 C MOVE A B C MOVE C D C MOVE E F C MOVE J K C MOVE L M C WHEN X=2 C .... C ENDSL /free select; when X=1; B = A; D = C; F = E; K = J; M = L; when X=2; ... endsl; /end-free ``` ```rpg Fix theme={null} C SELECT C WHEN X=1 C EXSR SR011 C WHEN X=2 C .... C ENDSL C ... C SR011 BEGSR C MOVE A B C MOVE C D C MOVE E F C MOVE J K C MOVE L M C ENDSR /free select; when X=1; exsr SR011; when X=2; ... endsl; /end-free ```

When using a PRINTER defined file, an INFDS keyword should be used to read the file’s line count from \*PSSR positions 367-368.

```rpg Bad theme={null} FCSO602R1 O E PRINTER USROPN or ... FCSO602R1 O E PRINTER USROPN F INFDS(WSFD01) F INFSR(*PSSR) D WSFD01 DS D W1ST01 *STATUS D W1SPNO 123 124B 0 ``` ```rpg Fix theme={null} FCSO602R1 O E PRINTER USROPN F INFDS(WSFD01) F INFSR(*PSSR) D WSFD01 DS D W1ST01 *STATUS D W1SPNO 123 124B 0 D W1LNCT 367 368B 0 D W1PGNO 369 372B 0 ```

When accessing a non-input-only file with a CHAIN/READx operation, the record is locked and becomes inaccessible to the system, and unreadable other programs. This is fine if you are about to update the record immediately, but if you have data processing to do following the read, it can leave the record unavailable for too long. Therefore, non-input files using the CHAIN, READ, READE, READP, and READPE operation codes should always be accessed with the no lock option, (N).

```rpg Bad theme={null} FDDDLJNL7 UF E K DISK INFSR(*PSSR) ... C READ DDDLJNL7 ``` ```rpg Fix theme={null} FDDDLJNL7 UF E K DISK INFSR(*PSSR) ... /free read DDDLJNL7; /end-free ```

While the OCCURS keyword still works, it is slower than its more modern replacement: DIM, which should be used instead.

```rpg Bad theme={null} D cussls ds Occurs(200) D CustNo 7p 0 D MonthlySls 11P 2 ``` ```rpg Fix theme={null} D cussls ds dim(200) D CustNo 7p 0 D MonthlySls 11P 2 ```

Initializing an alphanumeric field with the literal character ' ', '1', or '0' has the same effect as using the equivalent figurative constant \*BLANK, \*ON, or \*OFF, but the constant version removes ambiguity and makes the code more readable.

```rpg Bad theme={null} C MOVE ' ' FLD1 ``` ```rpg Fix theme={null} /free fld1 = ' '; /end-free ```

The use of options( \*nopass ) on a parameter makes it optional, so it should always be checked before use.

```rpg Bad theme={null} DCL-PR VIOLATION1 EXTPGM; LASTNAME Char(10); FIRSTNAME Char(10) OPTIONS(*NOPASS:*OMIT); END-PR; DCL-PI VIOLATION1; LASTNAME Char(10); FIRSTNAME Char(10) OPTIONS(*NOPASS:*OMIT); END-PI; dsply FIRSTNAME; dsply LASTNAME; // Noncompliant; LASTNAME is optional *inlr = *on; ``` ```rpg Fix theme={null} DCL-PR VIOLATION1 EXTPGM; LASTNAME Char(10); FIRSTNAME Char(10) OPTIONS(*NOPASS:*OMIT); END-PR; DCL-PI VIOLATION1; LASTNAME Char(10); FIRSTNAME Char(10) OPTIONS(*NOPASS:*OMIT); END-PI; dsply FIRSTNAME; IF %PARMS > 1; dsply LASTNAME; EndIf; *inlr = *on; ```

The use of positional notation can quickly become confusing, especially in a large or complex data structure. To keep the code clean and understandable, use length notation instead.

```rpg Bad theme={null} D PERSON DS QUALIFIED * Noncompliant D FName 1 30 D LName 31 60 ``` ```rpg Fix theme={null} D PERSON DS QUALIFIED D FName 30 D LName 30 ```

Prototypes should be used for program calls because their use allows compile-time checking of the number of parameters and the type and size of those parameters. Without the use of a prototype, that checking happens only at run-time.

A secondary benefit of the use of prototypes and CALLP, is that prototypes allow the caller to gain a clearer understanding of the data expected for each parameter because parameter names are purely descriptive.

```rpg Bad theme={null} ---- === Compliant solution [source,rpg] ``` ```rpg Fix theme={null} ```

Shared coding conventions allow teams to collaborate effectively. To improve readability, blank lines in a program should not have end-of-line comments.

```rpg Bad theme={null} I/COPY VALSRC,VALTEST FIX001 FIX001 ** Data structure for subroutine VALDS FIX001 I DS FIX001 I 5 60MMA FIX001 I 7 80DDA FIX001 * FIX001 I DS FIX001 I 5 60MMB FIX001 I 7 80DDB FIX001 ``` ```rpg Fix theme={null} I/COPY VALSRC,VALTEST FIX001 ** Data structure for subroutine VALDS FIX001 I DS FIX001 I 5 60MMA FIX001 I 7 80DDA FIX001 * FIX001 I DS FIX001 I 5 60MMB FIX001 I 7 80DDB FIX001 ```

The /EJECT compiler directive adds a page break when the code is printed, and should be used at the end of each F, D, and C specification section to make the structure of the code clearer and to enhance overall readability.

```rpg Bad theme={null} F* Conversion Job Activity File (CV) F* FCVJBAVP IF E K DISK F* Conversion Job Dependency File (CZ) F* FCVJBDPP IF E K DISK F* DP0RTCD S 1 DP0JBNM S 10A DP0ERMG S 100A DK0JBNM S LIKE(CVJBNM) DK1JBNM S LIKE(CVJBNM) DJST S 10A DIM(10) DI S 3 0 INZ C C EXSR SR999 C C EXSR SR100 ``` ```rpg Fix theme={null} F* Conversion Job Activity File (CV) F* FCVJBAVP IF E K DISK F* Conversion Job Dependency File (CZ) F* FCVJBDPP IF E K DISK F* F/EJECT DP0RTCD S 1 DP0JBNM S 10A DP0ERMG S 100A DK0JBNM S LIKE(CVJBNM) DK1JBNM S LIKE(CVJBNM) DJST S 10A DIM(10) DI S 3 0 INZ D/EJECT C C EXSR SR999 C C EXSR SR100 C/EJECT ```

SELECT and OMIT allow you to choose records from a logical file based on the values of specific fields. SQL views allow you to make the same distinctions, but their use is more efficient, and easier to read and understand. Therefore views are preferred over SELECT and OMIT statements.

```rpg Bad theme={null} Index: SKCOU03 *************** Beginning of data ************************************* A R SKCOU TEXT('Coupon Rate Details') A PFILE(SKCOUP) A K COUASS A K COUEFF DESCEND A S COUSTS COMP(EQ 'L') A S COUSTS COMP(EQ 'X') ****************** End of data *************************************** ``` ```rpg Fix theme={null} CREATE VIEW SKCOU03 AS SELECT A.COUFLD1, A.COUASS, A.COUEFF, A.COUSTS, A.COUFLD4 FROM SKCOUP A where A.COUSTS = ‘L’ OR A.COUSTS=’X’ Order by A.COUASS, A.COUEFF desc ```

Specifying the parameters to a procedure with a PLIST makes that procedure unusable from free-format code. Instead, prototypes should be used - both when defining a procedure and when calling it. They have the additional benefit of allowing you to use keywords such as Const to better-specify how parameters are passed to a procedure. Further, the use of a prototype instead of a PLIST is cleaner and more consistent with the code required for subprocedures.

```rpg Bad theme={null} * Noncompliant; PLIST specified C *ENTRY PLIST C PARM ZipCode 5 0 C PARM City 20 ... * Noncompliant; PLIST used in call C CALL 'OTHERPROG' C PARM ZipCode C PARM City ``` ```rpg Fix theme={null} D MYPROG PR D ZipCode 5I Const D City 20A Const D MYPROG PI D ZipCode 5I Const D City 20A Const ... D OTHERPROG PR ExtPgm('OtherProgram') D ZipCode 5I Const D City 20A Const /free OTHERPROG(ZipCode:City); /end-free ```

The use of a result data structure with file I/O improves performance because it moves the data in one large block from file to data structure (or vice versa) rather than field by field.

Additionally using a data structure can limit the problems caused by having bad data in a file. Without a data structure, the entire READ operation will fail at the first bad value. With one, the error comes only when the bad field is used.

```rpg Bad theme={null} F MyFile IF E Disk * Noncompliant C READ Record1 ``` ```rpg Fix theme={null} F MyFile IF E Disk D InputData DS LikeRec(Record1) C READ Record1 InputData ```

Indicators should not be used on "IF" statements because they foster poor logic flow and result in code that is difficult to read and maintain.

```rpg Bad theme={null} C 99FLD1 IFEQ FLD2 ``` ```rpg Fix theme={null} C FLD1 IFEQ FLD2 C *IN99 ANDEQ *ON ```

A results indicator should be used on a CHAIN statement because it leads to cleaner, more readable, and more correct code. Using an indicator results in the conditions record not found, and end of file being automatically handled correctly, without the need for further error handling.

```rpg Bad theme={null} C K0GMCP CHAIN SSGMCPR ``` ```rpg Fix theme={null} C K0GMCP CHAIN SSGMCPR 81 ```

Non-static variables initialized with INZ() are only reliably initialized on the first run through the code in a "terminate and stay resident" (TSR) program. Subsequent calls to the program within the same job do not re-initialize the variable with the value from INZ() unless the last record indicator is set at the end of the program.

Without setting the last record indicator, the assumptions the code makes about the variable’s initial value will be wrong every time but one, potentially leading to bad program behavior.

```rpg Bad theme={null} D TESTINZ PR extpgm('TESTINZ') D Parm1 15A D Parm2 15A CONST DMUTVAR S 15A INZ('ABC') // Noncompliant; *inlr not set D TESTINZ PI D Parm1 15A D Parm2 15A CONST /Free IF %PARMS > 1 and Parm2 <> ''; MUTVAR=Parm2; ENDIF; DSPLY(E) ('MUTVAR:' + MUTVAR); Parm1=MUTVAR; return; /End-free ``` ```rpg Fix theme={null} D TESTINZ PR extpgm('TESTINZ') D Parm1 15A D Parm2 15A CONST DMUTVAR S 15A INZ('ABC') D TESTINZ PI D Parm1 15A D Parm2 15A CONST /Free IF %PARMS > 1 and Parm2 <> ''; MUTVAR=Parm2; ENDIF; DSPLY(E) ('MUTVAR:' + MUTVAR); Parm1=MUTVAR; *inlr = *on; return; /End-free ```

QUALIFIED data structures result in cleaner code because you can’t reference the fields without using the qualifying name. They also allow you to have multiple sub-fields with the same name, meaning subfield names don’t have to be convoluted for uniqueness, and can be expressive instead.

```rpg Bad theme={null} * Noncompliant D Employee DS D EmpId 7P 0 D EFName 30A D ELName 30A D EPhone 11P 0 * Noncompliant D Contractor DS D CntId 7P 0 D CFName 30A D CLName 30A D CPhone 11P 0 /free EmpId = '000220'; /end-free ``` ```rpg Fix theme={null} D Employee DS QUALIFIED D Id 7P 0 D FName 30A D LName 30A D Phone 11P 0 D Contractor DS QUALIFIED D Id 7P 0 D FName 30A D LName 30A D Phone 11P 0 /free Employee.Id = '000220'; /end-free ```

While a file I/O call will compile without a file name, such code is almost always an error. The omission of the file name, can cause unexpected results in multi-file programs.

```rpg Bad theme={null} * Noncompliant C READ DS1 90 ``` ```rpg Fix theme={null} C READ PF0001 DS1 90 ```

Use of the LIKE statement for fields with the same specification that are being used together in the same statements makes the relevance of individual fields more understandable, and gives easy reference information, and makes relationships between those fields more obvious.

```rpg Bad theme={null} D NAME S 20A D PNAM S 20A ``` ```rpg Fix theme={null} D NAME S 20A D PNAM S LIKE(NAME) ```

The CONST keyword on a subprocedure’s parameter indicates that the parameter value will not be changed by the subprocedure. This is not just a nice way to communicate with the programmers who will call the procedure. It also offers performance benefits, because it allows the compiler to produce more optimized code. Further, using CONST means that a field of a similar data type will automatically be converted to the correct type and size for the parameter.

```rpg Bad theme={null} D X S 15A INZ('ABC') P SubProc1 B D SubProc1 PI D Parm1 15A // Noncompliant; read-only. Should be CONST D Parm2 15A /Free X = Parm1; Parm2 = X; return; /End-free P SubProc1 E ``` ```rpg Fix theme={null} D X S 15A INZ('ABC') P SubProc1 B D SubProc1 PI D Parm1 15A CONST D Parm2 15A /Free X = Parm1; Parm2 = X; return; /End-free P SubProc1 E ```

Development tools and frameworks usually have options to make debugging easier for developers. Although these features are useful during development, they should never be enabled for applications deployed in production. Debug instructions or error messages can leak detailed information about the system, like the application’s path or file names.

If a private field is declared but not used locally, its limited visibility makes it dead code.

This is either a sign that some logic is missing or that the code should be cleaned.

Cleaning out dead code decreases the size of the maintained codebase, making it easier to understand and preventing bugs from being introduced.

```rpg Bad theme={null} D I S 5P 0 INZ D* Noncompliant; X is not used in C-Specs D X S 5P 0 INZ C EVAL I = I + 1 ``` ```rpg Fix theme={null} ```

Using the same value on both sides of a binary operator is a code defect. In the case of logical operators, it is either a copy/paste error and, therefore, a bug, or it is simply duplicated code and should be simplified. In the case of bitwise operators and most binary mathematical operators, having the same value on both sides of an operator yields predictable results and should be simplified as well.

```rpg Bad theme={null} C IF X = X ``` ```rpg Fix theme={null} /free if a = a; // always true doZ(); endif; if a <> a; // always false doY(); endif; if a = b and a = b; // if the first one is true, the second one is too doX(); endif; if a = b or a = b; // if the first one is true, the second one is too doV(); endif; j = 5 / 5; //always 1 k = 5 - 5; //always 0 /end-free ```

There is no reason to re-assign a variable to itself. Either this statement is redundant and should be removed, or the re-assignment is a mistake and some other value or variable was intended for the assignment instead.

```rpg Bad theme={null} /free name = name; /end-free ``` ```rpg Fix theme={null} /free customer.name = name; /end-free ```

Duplicated string literals make the process of refactoring complex and error-prone, as any change would need to be propagated on all occurrences.

```rpg Bad theme={null} C MOVEL 'CLEAR' W1CLR Noncompliant C MOVEL '*DECOD' W1DCDE C MOVEL '*ERROR' W1ERR C MOVEL '*EXIT ' W1EXIT C MOVEL 'CLEAR' W1FIRT C MOVEL 'CLEAR' W1HELP ``` ```rpg Fix theme={null} /free W1CLR = 'CLEAR'; // Noncompliant W1FIRT = 'CLEAR'; W1HELP = 'CLEAR'; /end-free ```

SELECT \* should be avoided because it releases control of the returned columns and could therefore lead to errors and potentially to performance issues.

```rpg Bad theme={null} SELECT * FROM persons INTO newyorkers WHERE city = 'NEW YORK' ``` ```rpg Fix theme={null} SELECT firstname, lastname FROM persons INTO newyorkers WHERE city = 'NEW YORK' ```

Having two WHEN in the same SELECT statement or branches in the same IF structure with the same implementation is at best duplicate code, and at worst a coding error.

```rpg Bad theme={null} C IF X = 1 C EXSR SR01 C EXSR SR01 C ELSEIF X = 2 C EXSR SR02 C ELSEIF X = 3 C EXSR SR01 Noncompliant; duplicates first condition C EXSR SR01 C ENDIF ``` ```rpg Fix theme={null} ```