Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A "best-practice" coding rule for indentation size ? #616

Open
jm-c opened this issue Mar 18, 2022 · 3 comments
Open

A "best-practice" coding rule for indentation size ? #616

jm-c opened this issue Mar 18, 2022 · 3 comments

Comments

@jm-c
Copy link
Member

jm-c commented Mar 18, 2022

There is a consensus on the need to use indentation for if block and do loop block, with the main purpose to help following how the code works. But it's less clear that a "fixed indentation size" for all src code or even within each file is desirable.

  1. There are cases where keeping a list of similar S/R calls aligned might help maintaining the code, with no significant negative impact on readability, even when some of these calls are within different number of if block. This was mentioned there:
    Coding style example #550 (comment)
    but can occurs also in other files (e.g., read_pickup.F, write_pickup.F)

  2. Since MITgcm current code has many do loops (i,j,k,bi,bj, iTracer ...) with few if block around and a limitation on line length, we tend to use a smaller indentation size (often 1 or 2) than commonly used size 4 like in many other src code. But a size 1 indentation does not improve the readability of the code as much as larger indentation size (e.g. 2). Also, the loops on tile indices bi,bj as well as loops on horizontal indices i,j go by pair (there are barely any exception) so that in some src file a smaller indentation of 1 is used for these pairs of loops while if blocks keep the size 2 indentation; but it's not obvious that this makes the code harder to read.

  3. There is also the open question about how this coding rule will apply to existing code, new code and a mix of both (small addition to non-compliant existing code).

@timothyas
Copy link
Member

I think we should stick to an indentation of size 1 because I think that this strikes the right balance between readability and ye olde 72 character constraints. Also, based on my experience, this seems to be the most consistently used (please correct me if I'm wrong...).

Regarding the finer points:

  1. I think the S/R CALL ... statements should follow the indentation of the particular block they are in, no matter what. That way it is more clear what chain of events needs to happen for the S/R to be called.
  2. Yeah normally I opt for more spacing, but in the case of fortran 77 formatting I think 1 space strikes the balance. Just my take though...
  3. I think it should be enforced in new PRs, and we clean up old "offending" code as the PRs come in.

@mjlosch
Copy link
Member

mjlosch commented Mar 22, 2022

I like simple solutions. I have used indentations of 1 throughout for the reasons mentioned above (many loops and short lines). I have configured my editor to do this automatically, so I can go through a file and by hitting "tab" on each line fix the indentation automatically relative to the previous line. Some editors can also automatically indent regions or entire subroutine following fixed rules, which serves my laziness very well. That's why I like fixed rules (always the same indentation for a particular structure), but I am open to having different indentations for, say, do-loops and if-statements, if that makes any sense.

I prefer to follow strict indentation rules over aligning code vertically to support the call logic. But I don't feel too strongly about this

The current practice of fixing the formatting in routines that we change anyway appears to be useful to me. In general, we should encourage using any rules that we come up with rather than "enforce" them.

@timothyas
Copy link
Member

timothyas commented Mar 22, 2022

I agree with @mjlosch's framing of encouraging vs enforcing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants