ABINIT Discussion Forums

Hi,

Bad choices of variable names are sometimes harmful to the readability and portability of the code. The last one in date was the naming of a constant using the word 'const', which is also a C keyword. It caused the IBM cpp to replace it with an empty string and ended-up of course in xlf complaining about a syntax error.

I would thus like to start a list with the following:

• ban the word 'option' of allowed variable names, as it does not provide any information on what the variable does;
• ban French variable names;
• ban any keyword belonging to the C language or interpreted as an instruction by the C preprocessor;
• enforce variable names to be lower-case or mixed-case, while CPP macros should be upper-case only.

I don't feel it's too constraining, but I'd like to know what you think about it.

In any case, feel free to extend the list. Once it is comprehensive enough, there will be a corresponding test in the abirules series.

I agree heartily.

A possible addition is one-letter variable names (harder to check for though).

Matthieu

pouillon wrote:ban the word 'option' of allowed variable names, as it does not provide any information on what the variable does;

I completely agree. Using generic names renders the code difficult to follow
and it's already difficult to remember the innumerable arguments that are usually
passed to the abinit routines!
We might also use named constants to indicate the set of possible values of
option (ops I meant option_with_meaningful_name). Thus, instead of having:

if (option=1) then
elif (option=2) then

we can use

integer,parameter :: do_this = 1, do_that = 2

if (option == do_this) then

else if (option == do_that) then

I'm already using this convention when we test the value of optdriver, accesswf and other
GW internal variables. In this case both do_this and do_that are global parameters defined in defs_basis

pouillon wrote:enforce variable names to be lower-case or mixed-case, while CPP macros should be upper-case only.

Here I'm not so convinced. I usually use upper-case variables to indicate global names,
both for private and public variables defined in Fortran modules.
This convention makes it clear that the variable lives in another namespace thus changing its value
may affect the behavior of other parts of the code.
Fortran is not case-sensitive so I don't see the reason why we should enforce lower-case names.
Using well defined conventions for CPP macros --a set of allowed prefixes, no leading or trailing
underscores-- should suffice to avoid name clashing.

I'm not as optimistic as you are regarding name clashes. I think on the contrary that dedicating an exclusive "all upper-case" namespace to CPP options is not even sufficient. In particular, you cannot tell the developers of Autoconf how to name their macros, they decide for themselves. We should probably add rules and protocols about the use of these Autoconf macros as well, as it influences the portability and robustness of the build system.

We've witnessed it recently: you're the one who caused the builds on Fock to fail, by introducing the AC_C_CONST macro into the build system. As the IBM CPP is highly nonstandard, all occurrences of "const" where replaced by an empty string. You just escaped the ripple effects for a while because of a bug in the build system, which is now fixed. And at the end, I had to change the variable names myself, because my branch was failing, not yours.

If I made this proposal, it is not-at-all to restrict the freedoms of the developers. There are good reasons to be strict, since we are now dealing with more and more intricately interacting software components. We should decouple some of them as much as possible. And I don't think that using the same naming conventions for Fortran constants and CPP macros is a good idea.