tcoding_guidelines.rst - pism - [fork] customized build of PISM, the parallel ice sheet model (tillflux branch)
(HTM) git clone git://src.adamsgaard.dk/pism
(DIR) Log
(DIR) Files
(DIR) Refs
(DIR) LICENSE
---
tcoding_guidelines.rst (11570B)
---
1 .. default-role:: literal
2
3 PISM coding guidelines
4 ======================
5
6 .. contents::
7
8 File names
9 ----------
10
11 C++ source files use the extension `.cc`. Headers use `.hh`. C code uses `.c` and `.h`.
12
13 The *basename* of a file containing the source code for a class should match the name of
14 this class, including capitalization. For example, a class `Foo` is declared in `Foo.hh`
15 and implemented in `Foo.cc`.
16
17 Source and header files
18 -----------------------
19
20 Each header file must have an include guard. Do *not* use "randomized" names of include
21 guards.
22
23 Headers should *not* contain function and class method implementations unless these
24 implementations *should be inlined*; see below.
25
26 Inline functions and methods
27 ----------------------------
28
29 Implementations of inline methods should be *outside* of class declarations and in a
30 *separate header* called `Foo_inline.hh`. This header will then be included from `Foo.hh`.
31
32 Including header files
33 ----------------------
34
35 Include all system headers before PISM headers. Separate system headers from PISM headers
36 with an empty line.
37
38 Good:
39
40 .. code-block:: c++
41
42 #include <cassert>
43 #include <cstring>
44
45 #include "IceGrid.hh"
46
47 Bad:
48
49 .. code-block:: c++
50
51 #include <cstring>
52 #include "IceGrid.hh"
53 #include <cassert>
54
55 Whenever appropriate add comments explaining why a particular header was included.
56
57 .. code-block:: c++
58
59 #include <cstring> // strcmp
60
61 Naming
62 ------
63
64 Variable
65 ^^^^^^^^
66
67 - Variable names should use lower case letters and (if necessary) digits separated by
68 underscores, for example: `ice_thickness`.
69 - Do not abbreviate names of variables used in more than one scope **unless** this is
70 needed to keep the name under 30 characters. If a function uses a variable so many times
71 typing a long name is a hassle, create a reference with a shorter name that is only
72 visible in this scope. (This alias will be compiled away.)
73 - Single-letter variable names should only be used in "small" scopes (short functions,
74 etc.)
75 - If you are going to use a single-letter name, pick a letter that is easy to read (avoid
76 `i`, `l`, and `o`).
77 - Names of class data members should use the `m_` prefix, for example: `m_name`.
78 - Names of static data members should use the `sm_` prefix.
79 - Global variables (which should be avoided in general) use the `g` prefix.
80
81 Types and classes
82 ^^^^^^^^^^^^^^^^^
83
84 Names of types and classes use `CamelCase`.
85
86 Functions and class methods
87 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
88
89 Names of functions and class methods use the same rules are variable names, with some
90 additions.
91
92 - If a method is used to get a property of an object that cannot be reset (example:
93 `IceGrid::Mx()`), omit `get_` from the name.
94 - If a getter method has a corresponding setter method, their names should be
95 *predictable*: `Foo::get_bar()` and `Foo::set_bar()`. In this case, *do not* omit `get_`
96 from the name of the getter.
97
98 Namespaces
99 ----------
100
101 Everything in PISM goes into the `pism` namespace. See the source code browser for more
102 namespaces (roughly one per sub-system).
103
104 Notable namespaces include:
105
106 - ``atmosphere``
107 - ``bed``
108 - ``calving``
109 - ``energy``
110 - ``frontalmelt``
111 - ``hydrology``
112 - ``ocean``
113 - ``rheology``
114 - ``sea_level``
115 - ``stressbalance``
116 - ``surface``
117
118 Using directives and declarations
119 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
120
121 Do *not* import all names from a namespace with `using namespace foo;`
122
123 Do import *specific* names with `using ::foo::bar;` in `.cc` files.
124
125
126 Formatting
127 ----------
128
129 PISM includes a `.clang-format` file that makes it easy to re-format source to make it
130 conform to these guidelines.
131
132 To re-format a file, commit it to the repository, then run
133
134 .. code-block:: none
135
136 clang-format -i filename.cc
137
138 (Here `-i` tells clang-format to edit files "in place." Note that editing in place is
139 safe because you added it to the repository.)
140
141 Logical operators should be surrounded by spaces
142 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
143
144 .. code-block:: c++
145
146 // Good
147 if (a == b) {
148 action();
149 }
150
151 // Bad
152 if (a==b) {
153 action();
154 }
155
156 Commas and semicolons should be followed by a space
157 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
158
159 .. code-block:: c++
160
161 // Good
162 function(a, b, c);
163
164 // Bad
165 function(a,b,c);
166 function(a,b ,c);
167
168 Binary arithmetic operators should be surrounded by spaces
169 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
170
171 .. code-block:: c++
172
173 // Good
174 f = x + y / (z * w);
175
176 // Bad
177 f = x+y/(z*w);
178
179 Statements
180 ^^^^^^^^^^
181
182 One statement per line.
183
184 .. code-block:: c++
185
186 // Good
187 x = 0;
188 y = x + 1;
189
190 // Bad
191 x = 0; y = x + 1;
192
193 Code indentation
194 ^^^^^^^^^^^^^^^^
195
196 - Use two spaces per indentation level.
197 - Do not use tabs.
198 - Opening braces go with the keyword ("One True Brace Style").
199
200 Examples:
201
202 .. code-block:: c++
203
204 int function(int arg) {
205 return 64;
206 }
207
208 for (...) {
209 something();
210 }
211
212 class Object {
213 public:
214 Object();
215 };
216
217 Return statements
218 ^^^^^^^^^^^^^^^^^
219
220 Return statements should appear on a line of their own.
221
222 Do not surround the return value with parenthesis if you don't have to.
223
224 .. code-block:: c++
225
226 // Good
227 int function(int argument) {
228 if (argument != 0) {
229 return 64;
230 }
231 }
232
233 // Bad
234 int function(int argument) {
235 if (argument != 0) return(64);
236 }
237
238
239 Conditionals
240 ^^^^^^^^^^^^
241
242 - one space between `if` and the opening parenthesis
243 - no spaces between `(` and the condition (`(condition)`, not `( condition )`)
244 - all `if` blocks should use braces (`{` and `}`) *unless* it makes the code significantly
245 harder to read
246 - `if (condition)` should always be on its own line
247 - the `else` should be on the same line as the closing parenthesis: `} else { ...`
248
249 .. code-block:: c++
250
251 // Good
252 if (condition) {
253 action();
254 }
255
256 // Bad
257 if (condition) action();
258
259 // Sometimes acceptable:
260 if (condition)
261 action();
262
263 Loops
264 ^^^^^
265
266 `for`, `while`, `do {...} unless` loops are formatted similarly to conditional blocks.
267
268 .. code-block:: c++
269
270 for (int k = 0; k < N; ++k) {
271 action(k);
272 }
273
274 while (condition) {
275 action();
276 }
277
278 do {
279 action();
280 } unless (condition);
281
282 Error handling
283 --------------
284
285 First of all, PISM is written in C++, so unless we use a non-throwing `new` and completely
286 avoid STL, exceptions are something we have to live with. This means that we more or less
287 have to use exceptions to handle errors in PISM. (Mixing two error handling styles is a
288 *bad* idea.)
289
290 So, throw an exception to signal an error; PISM has a generic runtime error exception
291 class `pism::RuntimeError`.
292
293 To throw an exception with an informative message, use
294
295 .. code-block:: c++
296
297 throw RuntimeError::formatted(PISM_ERROR_LOCATION,
298 "format string %s", "data");
299
300 Error handling in a parallel program is hard. If all ranks in a communicator throw an
301 exception, that's fine. If some do and some don't PISM will hang as soon as one rank
302 performs a locking MPI call. I don't think we can prevent this in general, but we can
303 handle some cases.
304
305 Use
306
307 .. code-block:: c++
308
309 ParallelSection rank0(communicator);
310 try {
311 if (rank == 0) {
312 // something that may throw
313 }
314 } catch (...) {
315 rank0.failed();
316 }
317 rank0.check();
318
319 to wrap code that is likely to fail on some (but not all) ranks. `rank0.failed()` prints
320 an error message from the rank that failed and `rank0.check()` calls `MPI_Allreduce(...)`
321 to tell other ranks in a communicator that everybody needs to throw an exception.
322 (`pism::ParallelSection::failed()` should be called in a `catch (...) {...}` block
323 **only**.)
324
325 In general one should not use `catch (...)`. It *should* be used in these three cases,
326 though:
327
328 1. With `pism::ParallelSection` (see above).
329 2. In callback functions passed to C libraries. (A callback is not allowed to throw, so we
330 have to catch everything.)
331 3. In `main()` to catch all exceptions before terminating.
332
333 Performing an action every time a PISM exception is thrown
334 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
335
336 The class `pism::RuntimeError` allows setting a "hook" that is called by the constructor
337 of `RuntimeError`. See the example below for a way to use it.
338
339 .. code-block:: c++
340
341 #include <cstdio>
342
343 #include "error_handling.hh"
344
345 void hook(pism::RuntimeError *exception) {
346 printf("throwing exception \"%s\"\n", exception->what());
347 }
348
349 int main(int argc, char **argv) {
350
351 MPI_Init(&argc, &argv);
352
353 pism::RuntimeError::set_hook(hook);
354
355 try {
356 throw pism::RuntimeError("oh no!");
357 } catch (pism::RuntimeError &e) {
358 printf("caught an exception \"%s\"!\n", e.what());
359 }
360
361 MPI_Finalize();
362
363 return 0;
364 }
365
366 Calling C code
367 ^^^^^^^^^^^^^^
368
369 Check the return code of every C call and convert it to an exception if needed. Use macros
370 `PISM_CHK` and `PISM_C_CHK` for this.
371
372 When calling several C function in sequence, it may make sense to wrap them in a function.
373 Then we can check its return value and throw an exception if necessary.
374
375 .. code-block:: c++
376
377 int call_petsc() {
378 // Multiple PETSc calls here, followed by CHKERRQ(ierr).
379 // This way we need to convert *one* return code into an exception, not many.
380 return 0;
381 }
382
383 // elsewhere:
384 int err = call_petsc(); PISM_C_CHK(err, 0, "call_petsc");
385
386 Use assert() for sanity-checks that should not be used in optimized code
387 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
388
389 The `assert` macro should be used to check pre-conditions and post-conditions that can
390 fail *due to programming errors*.
391
392 **Do not** use `assert` to validate user input.
393
394 Note that *user input includes function arguments* for all functions and public members of
395 classes accessible using Python wrappers. (Use exceptions instead.)
396
397 Function design
398 ---------------
399
400 Functions are the way to *manage complexity*. They are not just for code reuse: the main
401 benefit of creating a function is that a self-contained piece of code is easier both to
402 **understand** and **test**.
403
404 Functions should not have side effects (if at all possible). In particular, do not use and
405 especially do not *modify* "global" objects. If a function needs to modify an existing
406 field "in place", pass a reference to that field as an argument and document this argument
407 as an "input" or an "input/output" argument.
408
409 Function arguments; constness
410 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
411
412 Pass C++ class instances by const reference *unless* an instance is modified in place.
413 This makes it easier to recognize *input* (read-only) and *output* arguments.
414
415 Do **not** use `const` when passing C types: `f()` and `g()` below are equivalent.
416
417 .. code-block:: c++
418
419 double f(const double x) {
420 return x*x;
421 }
422
423 double g(double x) {
424 return x*x;
425 }
426
427 Method versus a function
428 ^^^^^^^^^^^^^^^^^^^^^^^^
429
430 Do **not** implement something as a class method if the same functionality can be
431 implemented as a standalone function. Turn a class method into a standalone function if
432 you notice that it uses the *public* class interface only.
433
434 Virtual methods
435 ^^^^^^^^^^^^^^^
436
437 - Do not make a method virtual unless you have to.
438 - Public methods should not be virtual (create "non-virtual interfaces")
439 - **Never** add `__attribute__((noreturn))` to a virtual class method.
440
441 private versus protected versus public
442 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
443
444 Most data members and class methods should be `private`.
445
446 Make it `protected` if it should be accessible from a derived class.
447
448 Make it `public` only if it is a part of the interface.