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

why do we use STRSUC() instead of trim()? #407

Closed edwardhartnett closed 1 year ago

edwardhartnett commented 1 year ago

We have:

C> This subroutine removes leading and trailing blanks from a
C> character string.  The string may not contain any embedded blanks.
C>
C> @param[in]  STR1 -- character*(*): String
C> @param[out] STR2 -- character*(*): Copy of STR1 with leading and
C>                     trailing blanks removed
C> @param[out] LENS -- integer: Length of STR2
C>                     - -1 = STR1 contained embedded blanks
C>
C> @author J. Woollen @date 1994-01-06
      SUBROUTINE STRSUC(STR1,STR2,LENS)

How is this different from trim()?

jbathegit commented 1 year ago

FYI, as part of #406, I've already streamlined and modified strsuc so that it directly calls adjustl() and len_trim().

jbathegit commented 1 year ago

For the record, I also explained in #406 why I chose to keep strsuc as a subroutine. It's true that I could have just done away with strsuc altogether and just called adjustl and len_trim directly, but then I'd have to do that same thing in at least 10 different places throughout the rest of the library, so that's why I preferred to just modify strsuc to simplify it, but still keep it intact ;-)

edwardhartnett commented 1 year ago

OK, valx() has been removed that that means we don't have to spend any more time (i.e. NOAA's money) testing it, documenting it, distributing it, and, most significantly, it is one less subroutine for new programmers to understand in NCEPLIBS-bufr.

To remove an unneeded subroutine is going to be one of the most cost-effective uses of our time.

Looks like the only place that strsuc() is used is in strnum(). Is that correct?

If so, then let's remove strsuc() from the code, and use two lines of standard Fortran to replace it in strnum(), and we will have done a good day's work for NOAA. Our build, documentation, and testing tasks will all become easier at a stroke.

edwardhartnett commented 1 year ago

OK, if I do my find more carefully I find all the uses of strsuc():

find . -name '*.F90'|xargs grep -i strsuc
./strnum.F90:  call strsuc ( str, str2, lens )
./strsuc.F90:subroutine strsuc(str1,str2,lens)
./strsuc.F90:end subroutine strsuc
ed@koko:~/NCEPLIBS-bufr/src$ find . -name '*.f'|xargs grep -i strsuc
./dxdump.f:              CALL STRSUC(NEM(NC,1),WRK2,NCH)
./dxdump.f:                CALL STRSUC(WRK1,WRK2,NCH)
./dxdump.f:          CALL STRSUC(TABB(N,LUN)(96:98),WRK2,NCH)
./dxdump.f:          CALL STRSUC(TABB(N,LUN)(100:109),WRK3,NCH)
./dxdump.f:          CALL STRSUC(TABB(N,LUN)(110:112),WRK2,NCH)
./nemspecs.f:          CALL STRSUC( NEMO, TAGN, LTN )
./mtfnam.f:        CALL STRSUC ( TBLTYP, TBLTYP2, LTBT )
./mtinfo.f:      CALL STRSUC ( CMTDIR, MTDIR, LMTD )
./ufdump.f:           CALL STRSUC(NEMO,NEMO2,LNM2)
./ufdump.f:           CALL STRSUC(NEMO,NEMO2,LNM2)
./ufdump.f:               CALL STRSUC(NEMO,NEMO3,LNM3)
./hold4wlc.f:      CALL STRSUC( STR, MYSTR, LENS )
./gettagre.f:            CALL STRSUC( TAGRE, TAGTMP, LTRE )

OK, given the number of places this is used, I concede that it is pulling it's weight in the codebase. I will close this issue.