balewski's blog

Suggestion how to write programs which is faster to write, easier to debug, will last for years w/o maintenance, and is easy to extend (by other person).

* never use hardcoded numerical constants in particular in array dimensions, e.g. use:

enum{mx=4}; float xxx[mx];

const float pi=3.1417;

float arcLen=angle*pi;

instead of

float xxx[4];

float arcLen=angle*3.1417

because soon you will realize 5 params are actually needed and not all '4' in 1000-lines long code are the once you must change to '5' - by keeping dimension as named constant this task is easier. And the accuracy of pi you need will change with time.

* define everything only once per program. E.g. if 2 independent classes work on the same array declare its dimension in an independent header file included by both. Later you will change dimension of this array and forget to edit one of 7 headers - it will cost you a day to find who is writing over memory of other class member.

* declare array for anything what is similar and repetitive, e.g. use:

float par[mx];

instead of

float par0,par1,par2,par3;

this will allow the next step (below) and use e.g. of memset() to clear huge lists of variables very fast, pass large pieces of memory between methods.

* for repetitive operations always use indexed loops instead of copy/past of the same code

(this is the only thing where computer is better than a human: repeat some pre-programed task zillions times - use it)

e.g. use:

for(int i=0;i<mx;i++) doSth(i, par[[i]);

instead of:

doSth(0,par0);

doSth(1,par1);

doSth(2,par2);

It is guaranteed you will have at least one typo in the later version.

* keep a) local names short and b) global names long.

-For local names use:

float x,y,z,u,v;

v=x/y; u=z/y;

instead of

float particlemomentumx, particlemomentumy, particlemomentumz, particledirectionu , particledirectionv;

particledirectionu=particlemomentumy/particlemomentumz;

particledirectionx=particlemomentumu/particlemomentumz;

because it is hard to spot typos in long names used multiple times in close proximity. (do you see them?) .

The code is not your memory dump. Local names are descoped at the end of method so two x's from 2 different methods will never clash. It is not true for:

-For global (functions, class) names use

float totalEnergyFromBarrel()

rather than

float totEne()

because you work within collaboration and other people do not follow those rules - compiled code will pick the first function matching the name from the library (people ignore compiler & linker warnings) , certainly not your version. With long names chance of clash is smaller.

Never use as global name, class name, common block which are common: test, test1, energy, x,y, eta -other people do just that.

* always abort the code if you are not willing to handle exception.

(*** This rule do not apply to real-time applications handling e.g. detectors or rockets. ***)

E.g. if you constructed function returning location of an indexed object

float towerEta(int towerID) {

...

}

it is guaranteed it will be called with index out of sensible range. Abort code if index is out of range instead of returning zero (or returning memory from out of range location). This will force users of this function to check their code.

If you plan to handle the exception later put assert now and remove it later because you will get distracted.

If you think it will never happen you are wrong.

* if your code will be used (called) by others assume all possible wrong values of input arguments or sequence of calls will happen. Protect integrity of results delivered by your code by aborting if used not exactly as intended.

* assume any input you receive from an earlier code is corrupted in few %. Always check the range of float values, dimensions of input arrays. Make intelligent decision to skip bad values or abort whole code.

* write your code to mach to the input format rather than convert input file to another format with 3rd party software just so you can read it as you like. Say convert 2D array in to one vector. In the future you may need to do it 10,000 times - it is wast of time .

* in particular if you receive body of data from someone else always QA the input first. Do not event think about writing your main method until you are sure no junk is on input. You will save time. GIGO.

* be merciful for others and add 1 line of comment at least every 20 lines of code.

* use reversed logic in case of nested logical checks

A .and.B.and.C = not.( not.A .or. not.B .or. not.C)

Think the first but encode the later.

(*** does not apply to Python code ***)

E.g. use :

for(i=0;i<N;I++) {

if(x[i] <0) continue;

float y=sqrt(x);

if(y>20) continue;

float z=sqrt(20-y);

if( z <5) continue;

float u=....

.....

}

instead of :

for(i=0;i<N;I++) {

if(x[i] >0) {

float y=sqrt(x);

if(y<20) {

float z=sqrt(20-y);

if( z <5) {

float u=....

.....

}}}}

The later is hard to follow when code is long.

* always make compiler happy so it does not generate warnings. You are not smarter than ~1000 payed programers who wrote compiler - trust them you are missing sth. If you do not understand the warning it is even worse - it means you do not understand how compiler is interpreting your code. Change you code so you know what compiler is doing with it.

* Do not use StRoot classes if you can do what you need with root classes. E.g. TVector3() may be good enough vs. StThreeVectorF.

* Pay attention to "aa.h" vs. <aa.h>. Use :
#include "myClass.h" if you are controlling, changing this files
#include <somLibraryClass.h> if you never touch this file, has not even seen its source code.
In case of clash (or private version of a library) compiler will load your .h first when you want it.

* if you are writing 'quick' CERN root .C macro count lines. If it exceeds 300 lines stop doing that. Convert all your code to a regular C or C++, compile with make (or cons) and convert it to a regular executable. Cint is less smart than the compiler, it will not catch all your mistakes. You will win at the end a lot of time due to shorter debugging time.

* if you repeat multiple times similar set of operation on different variables write helper function doing this. Do not copy/past code and change name of variables. Later you will need to change this code and in one of the versions you will forget to change sth. FYI Multiple=2.

* monitor every variable you cut one and how much you loose on given cut, sth like this:

hA[0]=new TH1F("trStat","statistics for tracks",20,0.5,20.5);
hA[1]=new TH1F("trX","input track X@DCA; X(cm)",100,-5,5);
hA[2]=new TH1F("trY","input track Y@DCA; Y(cm)",100,-5,5);
.....
for (it=track.begin() ; it < track.end(); it++ ) {
TrackStump *t= &(*it);
hA[0]->Fill(1); // count tracks

hA[1]->Fill(t->r.x()); // monitor before cut
if(fabs(t->r.x()) >par_maxRxy) {
track.erase(it,it+1); continue;
}
hA[0]->Fill(2); // count tracks

hA[2]->Fill(t->r.y()); // monitor before cut
if(fabs(t->r.y()) >par_maxRxy) {
track.erase(it,it+1); continue;
}
hA[0]->Fill(3); // count tracks

* declare variable as local as possible. Value safety over CPU saving (on re-declaration declaration of variable).

good example: float sum2=0; for(i=0;i<10;i++) { float x=i+5; sum2+=x*x;}

bad example: float sum2=0,x; for(i=0;i<10;i++) {x=i+5; sum2+=x*x;}

Bad example allows to reuse temporary variable (value) outside the loop.

* use class names which describe its purpose. If you need to use word 'and' to do this it means your class does too much, split it on 2 classes.

----------- xxxxx---------

class B {
A *a;
}

cat B.cxx
#include "A.h"
B::B(){
a=new A;
a->x=5;
};

balewski's blog
Login or register to post comments

The STAR experiment

balewski's blog

User login

Navigation

How to write good code