---

1. Coding Style
   1. How MoleCuilder code should look like
      1. Declaration file
      2. Definition file
   2. One class, one file
2. Coding hints
   1. Some hints on good code vs. bad code
      1. end of stream checking
      2. Use of new/delete, return
      3. Use of return
      4. Specific (initial) values of variables
      5. Make use of declaring member variables and functions const
      6. Use forward declarations
      7. Use boost libraries
      8. Use the logger, not cout/cerr
      9. Use constructors initialization list properly
      10. Don't use #define for const values
      11. Copy and Assigment constructors are potentially dangerous
      12. Check for self-assignment

---

Coding Style

How MoleCuilder code should look like

Below you find a brief but hopefully complete list on how the code of espack should look like:

• identate by two spaces, never tabs
• Code Style from Eclipse, see ESPACK_codestyle.xml​:

Declaration file

Your declaration file should:

• contain all documentation (this is what the user may see in the code, never the implementation)
• contain include of config.h as very first
• always be properly bracketed in uniquely name preprocessor defines to avoid double inclusion.

/*
 * Point.hpp
 *
 *  Created on: <creation date>
 *      Author: <author>
 */

#ifndef POINT_HPP_
#define POINT_HPP_

// include config.h
#ifdef HAVE_CONFIG_H
#include <config.h>
#endif

/** Point is an implementation of a point in 2D space.
 *
 * With this we have a distance measure between two given coordinates.
 */
class Point
{
public:
  /** Constructor for class Point.
    * @param xc x coordinate
    * @param yc y coordinate
    */
  Point(double xc, double yc) :
    x(xc), y(yc)
  {}

  /** Returns distance to another Point \a other.
   *
   * @param other other Point
   * @return relative euclidian distance to \a other
   */
  double distance(const Point& other) const;

  //!> internal x coordinate of this Point
  double x;
  //!> internal y coordinate of this Point
  double y;
};

#endif /* POINT_HPP_ */

Definition file

Your definition file should:

• contain a disclaimer with copyright, the year is initially the current one and should be extended whenever the file is edited again.
• include first config.h then MemDebug.hpp, then your specific header file of this implementation, then all others

/*
 * Project: MoleCuilder
 * Description: creates and alters molecular systems
 * Copyright (C)  2010-2012 University of Bonn. All rights reserved.
 * Please see the LICENSE file or "Copyright notice" in builder.cpp for details.
 */

/*
 * \file Point.cpp
 *
 * This file contains Point definition.
 *
 *  Created on: <creation date>
 *      Author: <author>
 */

// include config.h
#ifdef HAVE_CONFIG_H
#include <config.h>
#endif

// always have the MemDebug first
#include "CodePatterns/MemDebug.hpp"

// then following by include of respective header file (without path)
#include "Point.hpp"

// then other includes
...
#include <math.h>
...

double Point::distance(const Point& other) const
{
  double dx = x - other.x;
  double dy = y - other.y;
  return sqrt(dx * dx + dy * dy);
}

One class, one file

Always have just one class implementation per file!. This allows for compiling the implementations in parallel and it is natural to split up the implementation with respect to the class structure. The same holds for the declaration in the header file.

Coding hints

Some hints on good code vs. bad code

Note that many of these hints are taken from the book of [Meyers, Effective C++].

end of stream checking

Using

   std::inputstream in;
   while (!in.eof()) {
     ..
   }

is bad. Rather one should use

   std::inputstream in;
   while (in.getline(...)) {
     ..
   }

or

   std::inputstream in;
   for(int j; in >> j;;) {
     ..
   }

or

   std::inputstream in;
   in >> j;
   if (in.fail())
    ..

This involves some extra typing but ensures that in case of faulty streams the error is properly pointed at.

Use of new/delete, return

The new and delete statements are written without brackets, they are not functions, i.e.

  new pointer;
  new array[3];
  delete pointer;
  delete[] array;

Use of return

The return statements should look like this

 return bar;
 return (foo && bar);

and not like

 return (bar);
 return(foo);

Specific (initial) values of variables

In general all variables always have to be initialized (we don't care about the extra tic, even if operating system guarantees zero as default memory value).

If they have to be set to some very small or very big value, use std::numeric_limits

  double smalldouble = std::numeric_limits<double>::min();
  double largedouble = std::numeric_limits<double>::max();

Also remember that there are also ::infinity() and alikes to set an illegal value which can be checked.

Make use of declaring member variables and functions const

We want to have a clean interface. Hence, it is vital to state in the interface that a function does not change the internal state of a class instance. If so, make it const.

The same holds for variables. If they are set in the constructor and only read afterwards, make them const.

Note: const variables are tricky with ​serialization, but in this specific case it is allowed to use const_cast<>() to allow writing a const member variable outside the constructor.

Note: If a function is const in nature but modifies a very specific variable (specific to the function but needs to be contained in the class scope, e.g. a counter how often the function has been called), make it mutable_.

Use forward declarations

Whenever possible use forward declarations in header files. They are vital in reducing compilation times. Remember that the preprocessor first compiles every include into the file subsequently served to the compiler. The larger this file becomes, the longer compilation takes.

If a function defined in a header file just has the parameter as a reference or as pointer, don't add the include but only the forward declaration in the header file.

// some header file

class MightyClass;

class SmallClass {
  ...
  void foo(MightyClass &_m);
  ...
};

Use boost libraries

Although in general, we do not want to rely to heavily on other libraries (because of licenses), do not reimplement the wheel if what you need is found in STL or ​boost which both are (quasi) standards.

Use the logger, not cout/cerr

Don't use std::cout or std::cerr (except maybe for internal tests which never make it into a commit).

Always use

#include "CodePatterns/Log.hpp"

  ...
  LOG(1, "INFO: Variable bla contains " << bla << ".");
  ...
  ELOG(1, "Variable is " << bla << " which is invalid!");
  ...

This way we can control the verbosity of the code easily and we do not end up with many commented-out wasteland alike ... std::cout << "Variable bla is " << ....

Use constructors initialization list properly

Constructors may be given an initialization list, e.g.

class SomeClass {
 ...
 double test;
 ...
};

SomeClass::SomeClass() :
  test(0.)
{}

SomeClass::SomeClass(const double t) :
  test(t)
{}

This is more efficient that setting the value in the constructor's body because the constructor of test gets called with the correct value right away instead of default constructor and then assignment operator.

Don't use #define for const values

Creating constant variables as

#define three 3.

is evil because:

• in the debugger you only see 3. not three. Hence, you have significantly less information and a harder time to understand the code.
• very strange errors may occur of a define is used as an argument in another define.

Instead use:

• Enum-Hack: enum {Num = 2}; for integers, used very frequently in the context of Template Meta Programming
• Const variables simply declare a (even global) const variable to contain the value.

Copy and Assigment constructors are potentially dangerous

Copy constructors such as

class A {
public:
    A();
    A(class A&);
    ~A();

   A& operator = (const class A&);
}

are dangerous to implement by hand and not leave it to the compiler, which generally does a good job. This is especially the case for derived classes with self-implemented copy constructors, where the copying of base class elements is often forgotten. Or when classes are extended by new member attributes and these are not included in the copy constructor as well.

See to it that unnecessary self-implemented copy constructors are removed and if ... implement a Init() function that both copy constructors use.

Check for self-assignment

If you have to implement a copy/assignment operator, always check for self-assignment!

SomeClass& SomeClass&::operator=(const SomeClass &instance) 
{
 if (*this  != instance) {
   ...
 }
}