OpenCMISS / iron

Source code repository for OpenCMISS-Iron
9 stars 62 forks source link

consolidating association & allocation checks to beginning of subroutines #62

Open SpiderMonkey1975 opened 7 years ago

SpiderMonkey1975 commented 7 years ago

Some subroutines have a fairly large number of association and allocation pointer checks that are implemented in a very deeply nested IF - ELSE IF structure. This can be hard to hard for developers and possibly difficult for compilers to optimize (when dealing with a high level of nesting).

A possible alternative is to conduct the checks in a series of sequential IF-THEN statements at the beginning of the subroutine. For instance:

if ( .not.ASSOCIATED(DOMAIN) ) call FlagError( "Domain is not associated.", ERR,ERROR, *998 )
    if ( .not.ASSOCIATED(DOMAIN%MAPPINGS) ) call FlagError( "Domain mappings is not associated.", ERR,ERROR, *998 )
    if ( .not.ASSOCIATED(DOMAIN%MAPPINGS%ELEMENTS) ) call FlagError("Domain mappings elements is not associated.",ERR,ERROR,*998)
    if ( .not.ASSOCIATED(DOMAIN%DECOMPOSITION) ) call FlagError( "Domain decomposition is not associated.", ERR,ERROR, *998 )
if ( .not.ASSOCIATED(DOMAIN%MESH) ) call FlagError( "Domain mesh is not associated.", ERR,ERROR, *998) 

Not an essential issue but maybe a coding style we could adopt as code is added and modified.

chrispbradley commented 7 years ago

Hi Mark, This is indeed the current code style (need link to code style here!) which says that guard clauses (what you have above) should be used. New code should be in the new code style and the old style should be converted as people work on routines/modules.

chrispbradley commented 7 years ago

Further to the above, the current plan is to also group them such that we can #ifdef out the input argument check guard clause block if required for performance reasons.