OpenEdge Coding Standards

Table of Contents

1. Objects
2. Error Handling
3. Data Access
4. Comments
5. Performance
6. Variables
7. Naming Conventions
8. Dynamic Objects
9. Code Styling
10. Other

Objects

Error Handling

<a name="no--error"></a><a name="2.1"></a>

• 2.1 NO-ERROR: Use NO-ERROR only when you expect an error to occur, and if you use it, handle error appropriately

  Why? NO-ERROR suppresses errors, which can cause database inconsistency issues, memory leaks, infinite loops and more...

  /* bad (error is suppressed, cMemberName is never assigned */
  ASSIGN iMemberNumber = INTEGER("ABC")
         cMemberName   = 'ABC' NO-ERROR.
      
  /* good (ver 1) - using structured error handling */
  ASSIGN iMemberNumber = INTEGER("ABC")
         cMemberName   = 'ABC'.
  /* ... some code... */
  CATCH eExc AS Progress.Lang.ProError:
    MESSAGE "Error:" + eExc:GetMessage(1).
  END.
      
  /* good (ver 2) - classic error handling (split safe assignment from unsafe) */
  ASSIGN cMemberName   = 'ABC'.
  ASSIGN iMemberNumber = INTEGER("ABC") NO-ERROR.
  IF ERROR-STATUS:ERROR THEN DO:
      /* handle error here */
  END.
  

<a name="no--error"></a><a name="2.2"></a>

• 2.2 BLOCK-LEVEL THROW Always use BLOCK-LEVEL ON ERROR UNDO, THROW statement

  Why? It changes the default ON ERROR directive to UNDO, THROW for all blocks (from default UNDO, LEAVE or UNDO, RETRY)

  Note: Use this parameter in legacy systems only. For new development use -undothrow 2 to set BLOCK-LEVEL ON ERROR UNDO, THROW everywhere

  Note: Use in new modules or when you're confident that the change in existing code is not going to break error handling

  /* bad (default ON ERROR directive used) */
  RUN internalProcedure.
  
  CATCH eExc AS Progress.Lang.AppError:
      /* this will never be executed */
  END.
      
  PROCEDURE internalProcedure:
      UNDO, THROW NEW Progress.Lang.AppError('Error String', 1000).
  END.
  
  /* good */
  BLOCK-LEVEL ON ERROR UNDO, THROW.
  
  RUN internalProcedure.
  
  CATCH eExc AS Progress.Lang.AppError:
      /* error will be caught here */
  END.
  
  PROCEDURE internalProcedure:
      UNDO, THROW NEW Progress.Lang.AppError('Error String', 1000).
  END.
  
  

  /* bad (routine-level doesn't cover FOR loops) */
  ROUTINE-LEVEL ON ERROR UNDO, THROW.
  RUN internalProcedure.
      
  CATCH eExc AS Progress.Lang.AppError:
      /* this will never be executed */
  END.
      
  PROCEDURE internalProcedure:
      FOR EACH bMemberRecord NO-LOCK:
          IF bMemberRecord.memberDOB < 01/01/1910 THEN
              UNDO, THROW NEW Progress.Lang.AppError('Found member with invalid DOB', 1000).
      END.
  END.
  
  /* good */
  BLOCK-LEVEL ON ERROR UNDO, THROW.
  
  RUN internalProcedure.
  
  CATCH eExc AS Progress.Lang.AppError:
      /* error will be caught here */
  END.
  
  PROCEDURE internalProcedure:
      FOR EACH bMemberRecord NO-LOCK:
          IF bMemberRecord.memberDOB < 01/01/1910 THEN
              UNDO, THROW NEW Progress.Lang.AppError('Found member with invalid DOB', 1000).
      END.
  END.
  

<a name="no--error"></a><a name="2.3"></a>

• 2.3 CATCH/THROW Use CATCH/THROW instead of classic error handling (NO-ERROR / ERROR-STATUS).

  Why? One place to catch all errors.

  Why? No need to handle errors every time they occur. No need to return error as output parameter and then handle them on every call.

  Why not? When working with FIND use NO-ERROR and check buffer availability

  /* bad */
  RUN myCheck (OUTPUT cErrorMessage) NO-ERROR.
  IF ERROR-STATUS:ERROR THEN
      MESSAGE "Error: " + ERROR-STATUS:GET-MESSAGE(1).
  ELSE IF cErrorMessage > '' THEN
      MESSAGE "Error: " + cErrorMessage.
      
  PROCEDURE myCheck:
      DEFINE OUTPUT PARAMETER opcStatusMessage AS CHARACTER NO-UNDO.
      
      IF NOT CAN-FIND (FIRST bMember) THEN DO:
          ASSIGN opoStatusMessage = 'Can not find member, try again'.
          RETURN.
      END.
  END.
      
  /* good (any application or system error will be caught by CATCH block) */
  RUN myCheck.
      
  CATCH eExc AS Progress.Lang.ProError:
      MESSAGE "Error: " + eExc:GetMessage(1).
  END.
      
  PROCEDURE myCheck:
      IF NOT CAN-FIND (FIRST bMember) THEN
          UNDO, THROW NEW Progress.Lang.AppError('Can not find member, try again', 1000).
  END.
  
  

<a name="catch--many"></a><a name="2.4"></a>

• 2.4 CATCH MANY Use multiple catch blocks if you need to handle errors differently based on error type (only if you need to handle errors differently)

  Note: If you have multiple catch blocks put most specific first and then the most generic. If the first catch matches the exception, it executes, if it doesn't, the next one is tried

  /* bad (one catch - multiple error types) */
  ASSIGN iMemberId = INTEGER(ipcParsedMemberId).
  FIND FIRST bMember NO-LOCK
       WHERE bMember.memberId = iMemberId NO-ERROR.
  IF NOT AVAILABLE bMember THEN
      UNDO, THROW NEW Progress.Lang.AppError('Invalid Member Id', 1000).
      
  CATCH eExc AS Progress.Lang.Error:
      IF TYPE-OF(eExc, Progress.Lang.AppError) THEN
          RETURN 'Invalid Application Error: ' + eExc:GetMessage(1).
      ELSE
          RETURN 'Invalid System Error: ' + eExc:GetMessage(1).
  END.
      
  /* good */
  ASSIGN iMemberId = INTEGER(ipcParsedMemberId).
  FIND FIRST bMember NO-LOCK
       WHERE bMember.memberId = iMemberId NO-ERROR.
  IF NOT AVAILABLE bMember THEN
      UNDO, THROW NEW Progress.Lang.AppError('Invalid Member Id', 1000).
      
  CATCH eExcApp AS Progress.Lang.AppError:
      RETURN 'Invalid Application Error: ' + eExcApp:GetMessage(1).
  END.
      
  CATCH eExcSys AS Progress.Lang.Error:
      RETURN 'Invalid System Error: ' + eExcSys:GetMessage(1).
  END.
  

  /* bad (multiple catch blocks - the same error handling) */
  FIND FIRST bMember NO-LOCK
       WHERE bMember.memberId = 123 NO-ERROR.
  IF NOT AVAILABLE bMember THEN
      UNDO, THROW NEW Progress.Lang.AppError('Invalid Member Id', 1000).
      
  CATCH eExcApp AS Progress.Lang.AppError:
      RETURN 'Error: ' + eExcApp:GetMessage(1).
  END CATCH.
      
  CATCH eExcSys AS Progress.Lang.Error:
      RETURN 'Error: ' + eExcSys:GetMessage(1).
  END CATCH.
  
  /* good */
  FIND FIRST bMember NO-LOCK
       WHERE bMember.memberId = 123 NO-ERROR.
  IF NOT AVAILABLE bMember THEN
      UNDO, THROW NEW Progress.Lang.AppError('Invalid Member Id', 1000).
  
  CATCH eExc AS Progress.Lang.Error:
      RETURN 'Error: ' + eExc:GetMessage(1).
  END CATCH.
  

<a name="no--error"></a><a name="2.5"></a>

• 2.5 RE-THROW Re-throw errors manually only if you need to do extra processing (like logging, or converting general error type to more specific) before error is thrown to upper level

  Why? Every procedure/class is supposed to change the default ON ERROR directive, forcing AVM to throw errors to upper level automatically

  /* bad */
  ASSIGN iMemberId = INTEGER('ABC_123').
  
  CATCH eExc AS Progress.Lang.ProError:
      UNDO, THROW eExc.
  END CATCH.
      
  /* good (convert General error into ParseError) */
  ASSIGN iMemberId = INTEGER('ABC_123').
  
  CATCH eExc AS Progress.Lang.ProError:
      UNDO, THROW NEW Mhp.Errors.ParseError(eExc:GetMessage(1), eExc:GetNumber(1)).
  END CATCH.
      
  /* good (write log message and throw error - use only at top most level) */
  ASSIGN iMemberId = INTEGER('ABC_123').
  
  CATCH eExc AS Progress.Lang.ProError:
      logger:error(eExc).
      UNDO, THROW NEW Mhp.Errors.ParseError(eExc:GetMessage(1), eExc:GetNumber(1)).
  END CATCH.
  

Data Access

<a name="record--locking"></a><a name="3.1"></a>

• 3.1 Never use SHARE-LOCK: Always use either NO-LOCK or EXCLUSIVE-LOCK

  Why? If you don't specify locking mechanism, SHARE-LOCK is used. NO-LOCK has better performance over SHARE-LOCK. Other users can't obtain EXCLUSIVE-LOCK on record that is SHARE locked

  /* bad */
  FIND FIRST bMember
       WHERE bMember.id EQ 0.346544767...
  FOR EACH bMember:...
  CAN-FIND (FIRST bMember WHERE bMember.id EQ 0.346544767)...
  
  /* good */
  FIND FIRST bMember NO-LOCK
       WHERE bMember.id EQ 0.346544767...
  FOR EACH bMember NO-LOCK:...
  CAN-FIND (FIRST bMember NO-LOCK
            WHERE bMember.id EQ 0.346544767)...
  

<a name="exp-trans-scope"></a><a name="3.2"></a>

• 3.2 Transaction Scope: Always explicitly define the transaction scope and strong scope applicable buffer

  /* bad */
  FIND FIRST bProvider EXCLUSIVE-LOCK NO-ERROR.
  IF AVAILABLE bProvider THEN
      ASSIGN bProvider.name = 'New Provider':U.
  /* ... some code... */
  FOR EACH bMember EXCLUSIVE-LOCK:
      ASSIGN bMember.memberName = 'New member name':U.
  END.
  
  /* good (provider should be updated separately from members) */
  DO FOR bProvider TRANSACTION:
      FIND FIRST bProvider EXCLUSIVE-LOCK
           WHERE bProvider.id = 0.657532547 NO-ERROR.
      IF AVAILABLE bProvider THEN
          ASSIGN bProvider.name = 'New Provider':U.
  END.
  /* ... some code... */
  FOR EACH bMember NO-LOCK
     WHERE bMember.category = 0.17567323 TRANSACTION:
      FIND FIRST bMember2 EXCLUSIVE-LOCK
           WHER