ABAP Review Check List
This is a work in process. Please feel free to ask questions,
make suggestions & correct misinformation!
Carolyn Fuller fuller@mit.edu
Mandatory Checks
 Many of these checks can be done automatically by launching transaction  ZUTALORS  
 
I. Documentation
   - Program documentation
 
   
   - Function Module documentation
 
   
   - Include documentation
 
   
   - In source
   documentation
 
   
   - Beginning block
   of source code documentation
 
II. Naming Conventions
Variable Names
   - Selection options = S_ 
 
   
   - Internal tables (global) = GT_ 
 
   
   
  - Internal tables (local) = LT_ 
 
   
   - Constants = GC_ 
 
   
   - Constants (local) = LC_
 
   - Ranges (global) = GR__
 
   
   - Ranges (local) = LR_
 
   - Form parameter  (Used
   in the actual form not the Perform statement.)
   
   - Single value or variable = PV_
 
   - Single structure or record(however complicated) = PS_
 
   - Internal table(however complicated the line structure) = PT_
 
   
    
    - Locally defined classes = LCL_
 
    - Locally defined interfaces = LIF_
 
  - Screen Parameters = A_ or P_
   
    
Program & Function Group Names
    
  - http://web.mit.edu/fss/dev/devstand5.html#ABAP
    Reports & Appendix
   B
 
  - http://web.mit.edu/fss/dev/devstand5.html#Function
    Groups & Appendix
   B
 
  - http://web.mit.edu/fss/dev/devstand5.html#Function
    Modules & Appendix
   B
 
  - http://web.mit.edu/fss/dev/devstand5.html#Module
    Pools (Dialog Programs)
 
 
    
    ABAP programs (reports), function groups and module pools all
      use a 2 character application name abbreviation. The list of
      current M.I.T. applications will be maintained in Appendix
    B.
   
   - Authorization Group in attributes
   - Programs that can be executed by all MIT users must be
   configured using the Authorization Group 'ZOPN2ALL' for
   Application '*'.
 
   
   - Development Class needs to be application specific. This
   standard is a work in process.
 
   - Unicode Check Active is check-marked.
 
IV. Function Modules
  -   Function Module interface parameters
    
      - The first letter should indicate the direction in which the parameter was passed:
        Input or importing = I
        Output or exporting = E
        Bi-directional or changing = C
 
      - 	The second letter should indicate the nature of the formal parameter:
        Single value or variable = V
        Single structure or record(however complicated) = S
        Internal table(however complicated the line structure) = T
 
    
   
  - 	Interface tables for a Function module should be defined as TYPE Table.
 
V. RFC for Web Applications
  - Names within structures used in an RFC interface and parameter names should be in English and not German. (e.g., COMPANY_CODE not BUKRS)
 
  - RFCs that perform searches on descriptive type text (e.g., Names or Description) must implement the IDD team's standard on how such searches should behave. This can be accomplished by utilizing the function module Z_UT_SPLIT_SEARCH_TERMS which builds a range table to be used in the select statement or table to be used in a string comparison. ( The IDD team's standard is documented http://userdocs.mit.edu/portal-nb/insidemitstandards/WebStandards/webSiteLandF/LandFSearch.html )
 
  - No special conversions should be done on Date, Time, Integer or Decimal fields. JavaEE can and should handle all data types.
 
  - Since SAP centric data rules can change via SAP configuration, data massaging should be done in the RFC and neither the end user nor the JavaEE developer should worry about SAP centric formatting. For instance, before a G/L Account is sent to the web leading zeros should be stripped. When it comes from the web and before it is used to update SAP the leading zeros should be put back. This includes fields that don't always have appropriate SAP exit routines such as Profit Center. Z_CA_CONVERT_PROFITCENTR_INPUT can be used to properly massage Profit Center data entered via web.
 
  - When ABAP EXCEPTIONS are raised the default behavior in our web applications is to send the user to a "fail" page where the displayed message will include the EXCEPTION and the EXCEPTION short text. Therefore if a custom MIT RFC implements an EXCEPTION, the name and short text for the EXCEPTION should be informative (e.g., SAFO_AUTHORIZATION_ERROR: You are not authorized for any SAFO report). If the developers do not want the web application to send the user to the "fail" page, the RFC developer should either inform the Java developer that the exception should be handled with an exception handler or the RFC developer should use a BAPIRET2 structure to hold the error, its message and its variables.
 
  - Use a table type of BAPIRET2 for building all error message tables (e.g., BAPIRET2_T). If another structure is used and it does not contain the same column names of interest (TYPE, MESSAGE, ID, NUMBER, MESSAGE_V1, MESSAGE_V2, MESSAGE_V3, MESSAGE_V4), the Java developer must be informed and special code will need to be written to handle the different structure.
 
  - Since there are times an error message sent from an RFC must be overridden by the web application make sure all these BAPIRET2 fields are appropriately populated (TYPE, MESSAGE, ID, NUMBER, MESSAGE_V1, MESSAGE_V2, MESSAGE_V3, MESSAGE_V4).
 
  - Since our web utility tool, mortar 4.10 & higher, will appear in all of our future SAP web applications and it is working with an error table named ET_RETURN, this name is a reserved name for error messages tables.
 
  - If a developer uses a name for their errors other than ET_RETURN or ET_MESSAGES, they should tell the JavaEE developer so that the new name can be configured. Then this new name cannot be used for anything other than errors by any other RFC being called by that web application.
 
  - If a developer uses the name ET_MESSAGES for anything other than error messages they should tell the JavaEE developer so that this name is removed from the application's default configuration.
 
  - For performance reasons, IMPORT/EXPORT or CHANGE parameters should never be defined as TYPE table for RFCs.
 
VI. Return Codes
  - Test return codes (sy-subrc) for success and failure after any I/O and calls to function modules (database selects, internal table reads, call transactions, I/O to UNIX or workstation files, etc.)
  
 
  VII. Transaction Code assigned to executable programs
 Every type "executable program" must have an assigned transaction code which is used to execute the program. 
  VIII. Authorization Checks for HR custom programs
  Methods to accomplish authorization checks for HR are illustrated below:
  - Use function module "HR_READ_INFOTYPE" instead of direct SELECT 
statements when reading a specific infotype.
 
  - Use logical database PNP to leverage SAP authorizations (caveat: 
performance can be slow).
 
  - SELECT statements should only be used when the SAP documented data 
interfaces which incorporate the SAP authorization checks (Logical 
DataBases, function modules, and BAPI's) cannot provide the functionality 
required. If it is necessary to use SELECT's, then you must perform your 
own AUTHORITY-CHECK on the data selected.
 
  - Place a strict authorization group at the program (transaction) 
level. If a wide variety of data for a large group of individuals is
needed in a single program, then this program must have a very strict 
authorization on who can run it (definitely not on any menu path).
   
    
IX. Parameter, P_BOUNCED_EMAIL, should be passed to Z_SENDMAIL
      In order of preference, please provide one of the following for P_BOUNCED_EMAIL :
  - A mailing list of people from the business process side who can recognize and have the ability to resolve the issue of a bounced mail (e.g. CAO maintains a master email list of cost object approvers).
 
  - A mailing list of people from an Admin Computing support or project team who can resolve the bounced mail.
 
  - "sap-bounces@mit.edu". This mail will go to Carolyn Fuller, David Rosenberg, Wai-ming Li, and Kevin Lyons for resolution. 
 
      - 	PBO 
 
      - 	PAI 
 
      - 	Global data 
 
      - 	Forms 
 
    XI. Application and Database Performance
    
      -  	Check each “select” statement for the use of index. This can be most easily determined using the Code Inspector, transaction SCI, which will report on any “select” statement against large tables not using as index. 
 
      - 	Check that there is no assumed sort order after the “select” statement. Do not assume that the data will be returned in primary key order.
 
      - 	Code Inspector, transaction SCI, should be used to spot meaningful errors, but the reviewer should use judgment to filter out meaningless errors.
 
    XII. Standards
    
      -           	Development 
 
      - 	Quality Assurance 
 
      - 	Change Request 
 
    Strongly Recommended Practices
          I. Non Database Performance
   - Dead Code (Program -> Check -> Extended Prog.
   Check) - unused subroutines appear as warnings under PERFORM/FORM
   interfaces. - unused variables appear as warnings under Field
   attributes. Transaction code is SLIN. This will also catch
   literals (section III below).
 
   
   - When possible use MOVE instead of
   MOVE-CORRESPONDING (move bseg to *bseg or move
   t_prps[] to t_prps2[] if you want to copy entire
   table or t_prps to t_prps2 if you only want to copy header
   line.)
 
   
   - Code executed more than once should be placed in a form
   routine.
 
   
   - SORT and READ
   TABLE t_tab WITH KEY ... BINARY SEARCH when possible
   especially against non-buffered
   table (Data Dictionary -> Technical Info)
 
   
   - SORT tables BY
   fields
 
   
   - Avoid unnecessary moves to
   table header areas.
 
   
   - Subroutine parameters should
   be typed for efficiency and to help prevent coding and runtime
   errors.
 
II. Database Performanc
  - Avoid ORDER BY unless there is index on the columns - sort internal 
    table instead
 
  - SELECT SINGLE 
    when possible
 
  - SELECT fields FROM 
    database table INTO TABLE t_tab (an internal 
    table) - Lengthy discussion.
 
  - Views (inner join) are a fast way to access 
    information from multiple tables. Be aware that the result set only includes 
    rows that appear in both tables.
 
  - Use subqueries 
    when possible.
 
  - "FOR 
    ALL ENTRIES IN..." (outer join) are very fast but keep in the mind 
    the special features and 3 pitfalls 
    of using it.
    (a) Duplicates are removed from the answer set as if you had specified 
    "SELECT DISTINCT"... So unless you intend for duplicates to be deleted 
    include the unique key of the detail line 
    items in your select statement. In the data dictionary (SE11) the fields belonging 
    to the unique key are marked with an "X" in the key column.
    (b) If the "one" table (the table that appears in the clause FOR ALL ENTRIES 
    IN) is empty, all rows in the "many" table (the table that appears in the 
    SELECT INTO clause ) are selected. Therefore make sure you check that 
    the "one" table has rows before issuing a select with the "FOR ALL ENTRIES 
    IN..." clause.
    (c) If the 'one' table (the table that appears in the clause FOR ALL ENTRIES 
    IN) is very large there is performance degradation 
    Steven Buttiglieri created sample 
    code to illustrate this. 
  - Where clause should be in 
    order of index See example.
    This is important when there are multiple indexes for a table and you want 
    to make sure a specific index is used. This will change when we convert from 
    a "rules based" Oracle optimizer to a "cost based" Oracle optimizer. You should 
    be aware of a bug in Oracle, lovingly referred to as the "3rd 
    Column Blues". Click here for 
    more information on indexes. 
  - Where clause 
    should contain key fields in an appropriate db index or 
    buffered tables. 
    As long as we are using the Oracle Cost Based Optimizer, be aware fo the "Third 
    Column Blues", an Oracle bug.
 
  - Avoid nested SELECTs (SELECT...ENDSELECT within another SELECT...ENDSELECT). 
    Load data in internal tables instead. See item 3 above.
 
  - Use SQL statistical functions 
    when possible (max, sum, ...)
 
  - Delete all rows from a table. A where 
    clause is mandatory. Specifying the client is the most efficient way.
 
  - Put Check statements into where clause 
    - caveat: Make sure that the index is still being used after you add the additional 
    selection criteria. If the select statement goes from using an index to doing 
    a db scan (reading each row in the database without going through an index) 
    get it out of the where clause and go back to using "Check"!
 
III. Literals
   - Codes ('MD') should use contants (c_medical)
 
   
   - Longer text should use
   text elements. Sample code is a good example because it uses
   the text element in conjunction with the hard coded text. This
   documents the text element and provides for the possibility of
   multi-language support.
 
IV. Miscellaneous
   - Use CASE statement
   instead of IF...ELSEIF when possible (It is only possible in
   equality tests)
 
   
   - Nested If -
   encounter most likely to fail first (specific to general)
 
   
   - And - encounter most
   likely to fail first (specific to general)
 
   
   - OR's - encounter most
   likely to succeed first (general to specific)
 
   
   - Variables should use Like when possible
 
   
   - Subroutine usage - don't
   place decision to execute in the subroutine
 
   
   - If not ( t_prps[] is initial ) (instead of
   describe table t_prps lines sy-tfill, if sy-tfill > 0...)
 
   
   - New document types confirmed with the configuration team via
   MIT-ABAP mail list prior to coding a report to access the
   data.
 
   
   - Dates need to be
   properly formatted using the user's default settings.
   For
   the explanation of the BDC example check out the developer's
   standards.