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.