-
Notifications
You must be signed in to change notification settings - Fork 265
Coding Guidelines
This page is the collection of some recommendations while working on LIGGGHTS code.
We highly recommand to read the Google C++ Style Guide. It provides good and profound recommandations for coding C++. In particluar you should keep the following sections in mind:
- LIGGGHTS uses 4 spaces for indentation. Do not use TABs or any other number of spaces. All editiors allow you to edit the default indentation behavior.
- Some legacy files use 2 spaces for indentation as they come from LAMMPS
- We now enforce Unix newlines in our repository
-
declare variables in the scope where you need them. Do NOT use the C-style of declaring all variables at once at the beginning of a function.
-
delete variables you no longer need (run your code with valgrind on small examples to check for memory leaks)
-
if you declare a variable, also initialize it with a sensible value.
// WRONG
void a_function()
{
int a,b,c,d,e,f;
d = 0;
for(a = 0; a < 10; a++)
{
b = a * 2;
if(b > 4)
{
c = b * 5;
d += (a + b + c);
}
}
}
// RIGHT
void a_function()
{
int d = 0;
for(int a = 0; a < 10; a++)
{
int b = a * 2;
if(b > 4)
{
int c = b * 5;
d += (a + b + c);
}
}
}- if you compute or define a value that never changes, or store a pointer which never changes, use const to give that information to the compiler
// WRONG
int length = sqrt(x*x + y*y + z*z); // computed once, then never changes
// RIGHT
const int length = sqrt(x*x + y*y + z*z);// WRONG
double * radius = atom->radius; // pointer is stored locally, but never manipulated
double ** x = atom->x;
// RIGHT
double * const radius = atom->radius;
double * const * const x = atom->x;
// HINT: const always binds to the thing to the left. read more complicated types from RIGHT to LEFT.
// E.g. double * const * const = a constant (const) pointer (*) to a constant (const) pointer (*) of doubleThere are several reasons why we want to enforce this. For one, you're giving the compiler useful information what you will be doing with the variable. It can then optimize certain code paths. This can decide if a variable is placed in memory or in a register. Second, it reduces the chances of abandoned variables over time and manipulating varables which were never meant to be editable.
LAMMPS and LIGGGHTS have continously grown over the last years and were manipulated by many different people. As with any code, sins of the past got picked up as "best practice", leading to copy & pasting code without questioning what it does. The following is one example of such a bad code snippet, which unfortunetly has been picked up multiple times.
// WRONG
char **fixarg;
fixarg=new char*[9];
for (int kk=0;kk<9;kk++) fixarg[kk]=new char[30];
strcpy(fixarg[0],"gamma");
fixarg[1]="all";
fixarg[2]="property/atom";
strcpy(fixarg[3],"gamma");
fixarg[4]="scalar";
fixarg[5]="yes";
fixarg[6]="yes";
fixarg[7]="yes";
fixarg[8]="0";
fix_gamma_ = modify->add_fix_property_atom(9,fixarg,style);
delete [] fixarg;What's wrong with this code? It allocates memory for strings (char[30]), doesn't use it and does not free at the end. So it's a memory leak. Character strings "like this" are of type const char * and are stored during compilation in a memory region inside the binary. This means it's not necessary to allocate memory for them. All that is needed is to store the pointer of that string somewhere.
// RIGHT
const char *fixarg[9];
fixarg[0]="gamma";
fixarg[1]="all";
fixarg[2]="property/atom";
fixarg[3]="gamma";
fixarg[4]="scalar";
fixarg[5]="yes";
fixarg[6]="yes";
fixarg[7]="yes";
fixarg[8]="0";
fix_gamma_ = modify->add_fix_property_atom(9,const_cast<char**>(fixarg),style);Only if you build your own strings during runtime, you have to allocate enough memory for it. You still store a constant adress to it in your fix arguments. That's because if a fix uses your arguments in some other way, it MUST create a copy for itself. You as the caller don't worry about that. You create your arguments, pass them, and clean them up afterwards. If the fix used some of that data it must have a copy anyway.
// RIGHT
const char * newarg[12];
newarg[0] = "...";
newarg[1] = "...";
newarg[2] = "...";
newarg[3] = "...";
newarg[4] = "...";
newarg[5] = "...";
newarg[6] = "...";
newarg[7] = "...";
// create custom string at runtime
char * newarg8 = new char[30];
sprintf(newarg8,"%f",some_float);
newarg[8] = newarg8;
newarg[9] = "...";
newarg[10] = "...";
newarg[11] = "...";
modify->add_fix(12,const_cast<char**>(newarg));
// free custom string
delete [] newarg8;Fixes that are created within other fixes should be postfixed with a '', e.g. "fix internal_array all property/atom". If the fix contains output data which is accessed by the user through input scripts then no trailing underscore should be used, e.g. the pressure property/atom in fix powder/update.
- Prefer multiplications of divisions (slow: a/2.0; fast: 0.5*a)
- Avoid "pow()" (slow: pow(a,3.0); fast: a*a*a)
- Use the operators '+=','-=','=', and '/=', instead of the operators '+','-','', and '/'
- Simplify your equations on paper first (e.g., pre-compute constants by hand)
- Reduce expensive computations (exp, log) to a minimum by computing temporary variables.
- The rather expensive call to "sqrt()" can often be avoided, especially when you compare values (e.g., distances between particles)
- for more details search the web (e.g., this is a good page)
- Carefully check such that divisions by zero can never occur (e.g., implement and error statement, or add a small number)
- The body of the documentation text should not exceed 80 characters in width.
- Keywords should be highlighted using curly brackets.