From: Wolfgang Bangerth Date: Tue, 24 Aug 2010 11:54:05 +0000 (+0000) Subject: Have a section on defensive programming. X-Git-Tag: v8.0.0~5701 X-Git-Url: https://gitweb.dealii.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c37e504f12fca3e79d6125dabb9df5bd94b7bcea;p=dealii.git Have a section on defensive programming. git-svn-id: https://svn.dealii.org/trunk@21690 0785d39b-7218-0410-832d-ea1e28bc413d --- diff --git a/deal.II/doc/doxygen/headers/coding_conventions.h b/deal.II/doc/doxygen/headers/coding_conventions.h index 328d2d9985..29b76c0939 100644 --- a/deal.II/doc/doxygen/headers/coding_conventions.h +++ b/deal.II/doc/doxygen/headers/coding_conventions.h @@ -14,41 +14,45 @@ /** * @page CodingConventions Coding conventions used throughout deal.II -Throughout deal.II, we strive to keep our programming style and the kind -of interfaces we provide as consistent as possible. To this end, we -have adopted a set of coding conventions that we attempt to follow -wherever possible. When reading through them, it is important to -remember that they are not god given or better than any other set of -conventions; their purpose is merely to keep deal.II as uniform as -possible. Uniformity reduces the number of bugs we produce because we -can, for example, always assume that input arguments come before -output arguments of a function call. They also simplify reading code -because some things become clear already by looking at the style a -piece of code is written, without having to look up the exact -definition of something. - -The following are the general rules we attempt to follow: +Throughout deal.II, we strive to keep our programming style and the kind of +interfaces we provide as consistent as possible. To this end, we have adopted +a set of coding conventions that we attempt to follow wherever possible. They +have two parts: style issues, and something we call "defensive programming", +the latter being an attempt to let our code help us find bugs. When reading +through them, it is important to remember that styles are not god given or +better than any other set of conventions; their purpose is merely to keep +deal.II as uniform as possible. Uniformity reduces the number of bugs we +produce because we can, for example, always assume that input arguments come +before output arguments of a function call. They also simplify reading code +because some things become clear already by looking at the style a piece of +code is written, without having to look up the exact definition of something. + +

Style issues

  1. %Functions which return the number of something (number of cells, - degrees of freedom, etc) should start with n_*
  2. + degrees of freedom, etc) should start with n_*. Example: + SparsityPattern::n_nonzero_entries().
  3. %Function which set a bit or flag should start with set_*; - functions which clear bits of flags should be named clear_*
  4. + functions which clear bits of flags should be named clear_*. + Example: CellIterator::set_refine_flag(). -
  5. After each function, at least three empty lines are expected to +
  6. In the implementation files, after each function, at least three + empty lines are expected to enable better readability. One empty line occurs in functions to - group blocks of code, two empty lines are not enough to distinguish - visibly enough.
  7. + group blocks of code, two empty lines are not enough to visibly + distinguish sufficiently that the code belongs to two different functions.
  8. Whenever an integer variable can only assume nonnegative values, - it is marked as unsigned.
  9. + it is marked as unsigned. The same applies to functions that can only + return positive or zero values. Example: Triangulation::n_active_cells(). -
  10. Whenever an argument will not be changed, it should be marked +
  11. Whenever an argument to a function will not be changed, it should be marked const, even if passed by value. Generally, we mark input parameters as const. This aids as an additional documentation tool to clarify the - purpose of a parameter - and lets the compiler issue warnings if such a parameter variable is + intent of a parameter (input, output, or both) + and lets the compiler issue warnings if such a parameter is changed, which is often either involuntarily or poor style.
  12. Whenever a function does not change any of the member variable of @@ -69,12 +73,13 @@ The following are the general rules we attempt to follow: block of public functions, beginning with the constructors, then the destructors. If there are public member variables, these have to occur before the constructor. Public variables shall only be - used if constant or unavoidable.
  13. - + used if constant (in particular if they are static and constant) + or unavoidable. +
    After the public members, the protected and finally the private members are to be listed. The order is as above: first variables - then functions. - + then functions. +
    Exceptions shall be declared at the end of the public section before the non-public sections start. @@ -85,7 +90,8 @@ The following are the general rules we attempt to follow:
  14. Exceptions are used for %internal parameter checking and for consistency checks through the Assert macro. Exception handling - like done by the C++ language (try/throw/catch) are used to + like done by the C++ language (try/throw/catch, and using the + AssertThrow macro) are used to handle run time errors (like I/O failures) which must be on in any case, not only in debug mode.
  15. @@ -98,6 +104,269 @@ The following are the general rules we attempt to follow:
  16. Each class has to have at least 200 pages of documentation ;-)
  17. +
+ + +

Defensive programming

+ +

Defensive programming is a term that we use frequently when we talk about +writing code while in the mindset that errors will happen. Here, errors can +come in two ways: first, I can make a mistake myself while writing a +functions; and secondly, someone else can make a mistake while calling my +function. In either case, I would like to write my code in such a way that +errors are (i) as unlikely as possible, (ii) that the compiler can already +find some of the mistakes, and (iii) that the remaining mistakes are +relatively easy to find, for example because the program aborts. Defensive +programming is then a set of strategies that make these goals more likely. +

+ +

+Over time, we have learned a number of techniques to this end, some of which +we list here: +

    +
  1. Assert preconditions on parameters: People call functions with wrong + or nonsensical parameters, all the time. As the prototypical example, + consider a trivial implementation of vector addition: + +
    +    Vector &
    +    operator += (Vector       &lhs,
    +                 const Vector &rhs) {
    +    {
    +      for (unsigned int i=0; i
    +  
    +  While correct, this function will get into trouble if the two vectors
    +  do not have the same size. You think it is silly to call this function
    +  with vectors of different size? Yes, of course it is. But it happens
    +  all the time: people forget to reinitialize a vector, or it is reset in
    +  a different function, etc. It happens. So if you are in such an unlucky
    +  case, it can take a long time to figure out what's going on because
    +  you are likely to just read uninitialized memory, or maybe you are
    +  writing to memory the lhs vector doesn't actually own.
    +  Neither is going to lead to immediate termination of the program,
    +  but you'll probably get random errors at a later time. It would be
    +  much easier if the program just stopped here right away. The following
    +  implementation will do exactly this:
    +  
    +  
    +    Vector &
    +    operator += (Vector       &lhs,
    +                 const Vector &rhs) {
    +    {
    +      Assert (lhs.size() == rhs.size(),
    +              ExcDimensionMismatch (lhs.size(), rhs.size());
    +      for (unsigned int i=0; i
    +  
    +  The Assert macro ensures that the condition is true
    +  at run time, and otherwise prints a string containing information
    +  encoded by the second argument and aborts the program. This way,
    +  when you write a new program that happens to call this function,
    +  you will learn of your error right away and have the opportunity
    +  to fix it without ever having to seriously debug anything.
    +  

    + As a general guideline, whenever you implement a new function, + think about the preconditions on parameter, i.e. what does the + function expect to be true about each of them, or their combination. + Then write assertions for all of these preconditions. This may be + half a dozen assertions in some cases but remember that each assertion + is a potential bug already found through trivial means. +

    + In a final note, let us remark that assertions are of course expensive: + they may make a program 3 or 5 times slower when you link it against + the debug version of the library. But if you consider your overall + development time, the ability to find bugs quickly probably far outweighs + the time you spend waiting for your program to finish. Furthermore, + calls to the Assert macro are removed from the program in optimized mode + (which you presumably only use once you know that everything runs just + fine in debug mode. The optimized libraries are faster by a factor of + 3-5 than the debug libraries, at the price that it's much harder to find + bugs. +

  2. + +
  3. Assert postconditions: If a function computes something + non-trivial there may be a bug in the code. To find these, use + postconditions: just like you have certain knowledge about useful values + for input parameters, you have knowledge about what you expect possible + return values to be. For example, a function that computes the norm of + a vector would expect the norm to be positive. You can write this as + follows: + +
    +    double norm (const Vector &v) {
    +    {
    +      double s = 0;
    +      for (unsigned int i=0; i= 0, ExcInternalError());
    +      return std::sqrt(s);
    +    }
    +  
    +
    + This function is too simple to really justify this assertion, but imagine + the computation to be lengthier and you can see how the assertion helps + you ensure (or hedge) yourself against mistakes. Note that one + could argue that the assertion should be removed once we've run the program + a number of times and found that the condition never triggers. But it's + better to leave it right where it is: it encodes for the future (and for + readers) knowledge you have about the function; if someone comes along + and replaced the implementation of the function by a more efficient + algorithm, the assertion can help make sure that the function continues + to do what it is supposed to do. +
  4. + +
  5. Assert internal states: In a similar vein, if you have a + complex algorithm, use assertions to ensure that your mental model + of what is going on matches what is indeed true. For example, assume + you are writing a function that ensures that mesh sizes do not change + too much locally. You may end up with a code of the following kind: + +
    +    for (cell=triangulation.begin(); ...)
    +      for (face=0; ...)
    +        {
    +          if (something)
    +            { ... }
    +          else
    +            {
    +                // we have a cell whose neighbor must
    +                // be at the boundary if we got here
    +            }
    +        }
    +  
    +
    + The conditions that got us into the else-branch may be + complicated, and while it may be true that we believed that the + only possibility we got here is that the neighbor is at the boundary, + there may have been a bug in our implementation. There may also have been + a bug in our thinking, or someone changes the code way above in the same + function and forgets about the issue here, or a change at a completely + different location in the library makes the assumption untenable. In + all of these cases, the explicit statement of our assertion makes sure + that these problems are easily found. +
  6. + +
  7. Initialize variables at the point of their declaration if they + live on the stack: + Traditional C required that variables are declared at the beginning of + the function even if they are only used further below. This leads to + code like this that we may imagine in a 1d code: + +
    +    template @
    +    void foo ()
    +    {
    +      Point cell_center;
    +      ... // something lengthy and complicated
    +      for (cell = dof_handler.begin_active(); ...)
    +        {
    +          cell_center = (cell->vertex(0) + cell->vertex(1)) / 2;
    +          ...
    +        }
    +  
    +
    + The problem is that if the code between the declaration and initialization + is long and complicated, you can't look up on one page what the type of + a variable is and what it's value may be. In fact, it may not even be + quite clear that the variable is used initialized at all, or whether it + is accidentally left uninitialized. +

    + A better way to do this would be as follows: + +

    +    template @
    +    void foo ()
    +    {
    +      ... // something lengthy and complicated
    +      for (cell = dof_handler.begin_active(); ...)
    +        {
    +          Point cell_center = (cell->vertex(0) + cell->vertex(1)) / 2;
    +          ...
    +        }
    +  
    + + This makes it much clearer what the type of the variable is + and that it is in fact only ever used when initialized. Furthermore, + if someone wants to read the code to see what the variable is in fact + doing, declaring and initializing it in the innermost possible scope + makes this task easier: we don't have to look upwards for it beyond + the declaration, and we don't have to look downward beyond the end + of the current scope since this is where the variable dies. +

    + As a final note, it is clear that you can only do this sort of stuff + for variables that completely live on the stack without allocating memory + on the heap. Within deal.II, this is only true for builtin types like + int, double, char, etc, as well as the Point and Tensor + classes. Everything else has something like a std::vector + as a member variable, which requires memory allocation — you don't + want to declare these inside loops, at least not if the loop is + traversed frequently. + + +

  8. Make variables const: To pick up on the example above, note + that in most cases we will never change the variable so initialized + any more. In other words, if this is the case, we may as well write + things as follows: + +
    +    template @
    +    void foo ()
    +    {
    +      ... // something lengthy and complicated
    +      for (cell = dof_handler.begin_active(); ...)
    +        {
    +          const Point cell_center = (cell->vertex(0) + cell->vertex(1)) / 2;
    +          ...
    +        }
    +  
    +
    + By marking the variable as constant we make sure that we don't accidentally + change it. For example, the compiler could catch code like this: + +
    +        if (cell_center[0] = 0)
    +          ...
    +  
    +
    + This was most likely meant to be a == rather than an + assignment. By marking the variable as const, the compiler would have + told us about this bug. Maybe equally importantly, human readers of the + code need not look further down whether the value of the variable may + actually be changed somewhere between declaration and use — it + can't be if it is marked as const. +
  9. + +
  10. Make input arguments of functions const: The same essentially + holds true as well as for function arguments: If you have no intention + of changing a variable (which is typically the case for input arguments), + then mark it as constant. For example, the following function should take + its argument as a constant value: + +
    +     template 
    +     typename Triangulation::cell_iterator
    +     CellAccessor::child (const unsigned int child_no)
    +     {
    +       ...;
    +       return something;
    +     }
    +  
    +
    + Here, the user calls cell-@>child(3), for example. There really + is no reason why the function would ever want to change the value of + the child_no argument — so mark it as constant: + this both helps the reader of the code understand that this is an + input argument of the function for which we need not search below whether + it is ever changed, and it helps the compiler help us finding bugs if + we ever accidentally change the value.
*/