search

NOAA-EMC / NCEPLIBS-bufr

The NCEPLIBS-bufr library contains routines and utilites for working with the WMO BUFR format.
Other
44 stars 19 forks source link

use implicit none in all Fortran codes #328

Closed edwardhartnett closed 3 months ago

edwardhartnett commented 1 year ago

implicit none guards against one of the most common programming bugs: mispelling a varaible.

When we misspell a fortran variable, and a new one is created, it's extremely easy to introduce a bug. For example:

C           Store this message into MODULE MSGMEM.

            ICT = ICT + 1
            IF ( ( NDXM + ICT ) .GT. MXDXM ) GOTO 902
            IPDXM(NDXM+ICT) = LDXM + 1
            LMEM = NMWRD(MGWA)
            IF ( ( LDXM + LMEM ) .GT. MXDYW ) GOTO 903
            DO J = 1, LMEM
              MDX(LDXM+J) = MGWA(J)
            ENDDO
            LDXM = LDXM + LMEM

Where is the bug in this code? If it's obvious to you, it certainly is not to me, nor will it be to most programmers.

The good news is that the Fortran compiler will tell us exactly where the bug is, and will refuse to let us build the code at all, until we fix it. All we need to do is use implicit none to get this fantastic, automatic, and infallible bug finder.

edwardhartnett commented 1 year ago

I think we all agree that implicit none should be used in all new code (including tests). We will add implicit none to existing code after full testing, and as we refactor or change the code.

edwardhartnett commented 8 months ago

@jbathegit are we adding implicit none to all codes? When I look at the code I don't see them, even in subroutines where it would be really easy to put it there.

Do you not agree with this issue?

jbathegit commented 8 months ago

I do agree that this is a worthwhile goal, mostly for the reason you noted of more quickly finding a misspelled variable. I just haven't done any further work on this, because I've been prioritizing whatever time I have to work with NCEPLIBS-bufr on getting us to a more full level of testing, as you yourself noted above.

(And BTW, the bug in the above snippet is the misspelling MXDYW, which should be MXDXW :-)