Exit early to avoid waterfall code
ivan-pi opened this issue · 3 comments
From Recommendations To Write (Slightly More) Readable And (Thus) Robust Code 7:
Exit early to avoid waterfall code. Especially for longer code blocks, this minimizes cognitive overhead.
(originally a suggestion from @interkosmos posted at Discourse)
The current modularized minpack source code, includes sections for checking the input parameters like this one taken from lmder1
:
Info = 0
! check the input parameters for errors.
if (n > 0 .and. m >= n .and. Ldfjac >= m .and. Tol >= zero .and. &
Lwa >= 5*n + m) then
! call lmder.
maxfev = 100*(n + 1)
ftol = Tol
xtol = Tol
gtol = zero
mode = 1
nprint = 0
call lmder(fcn, m, n, x, Fvec, Fjac, Ldfjac, ftol, xtol, gtol, maxfev, &
& Wa(1), mode, factor, nprint, Info, nfev, njev, Ipvt, Wa(n + 1)&
& , Wa(2*n + 1), Wa(3*n + 1), Wa(4*n + 1), Wa(5*n + 1))
if (Info == 8) Info = 4
end if
Here in the "easy" driver there is only level of indentation, so it's not as problematic, but for the actual routines which do the work, getting rid of one level of indentation would make them easier on the eye.
The solution is to return early, per the advice above:
info = 0 ! flag for improper input parameters
! check the input parameters for errors
if (n < 1 .or. m < n) return
if (ldfjac < m) return
if (tol < zero) return
if (lwa < 5*n + m) return
! default settings
maxfev = 100*(n+1)
ftol = tol
xtol = tol
gtol = zero
mode = 1
nprint = 0
call lmder( ... )
if (info == 8) info = 4
Since Fortran does not short-circuit expressions I guess it also makes sense to separate the condition. (For correct input parameters, all the conditionals should be false, but in the case not, we might save a few evaluations and exit early.) This might also just by a personal preference of mine, that is I dislike long logical expressions and/or line continuation:
if (n < 1 .or. m < n .or. ldfjac < m .or. tol < zero .or. lwa < 5*n + m) &
return
Maybe we should also make the return code more concrete and have info
point to the incorrect argument rather than just raising a general input error (similar like done in Lapack).
I'm generally in favor of making the error codes more specific. The only downside I see is this will likely "invalidate" the current usage instructions and also won't necessarily be back compatible, requiring consumers to modify their programs; essentially we'd be breaking the API.
Any breaking changes would have to be communicated clearly.
We could introduce a compile time DEBUG
flag (via the preprocessor), which would produce verbose error output if enabled. This would not break the established error codes.