Hi BRATs,
Note: If this mail is similar to a previous mail, it's because I've
sendt it twice. The first time around it didn't seem to be distributed
by the list server. My apologies for the inconviniance.
I've updated BRAT so that it may compile on Windoze 98, using
Micros**t Visual F**k++ version 6.0, using ROOT version 2.25/03. I
used the makeNT.bat script in the top BRAT directory.
I managed to compile almost all libraries, except Monitor, Mult (see
more below), Raw, and Online. makeNT.bat now creates as many libraries
as I found possible, and copies the header files to include before
compiling.
Several changes was needed in the code.
---------------------------------------
The most prevaling one, has to do with the s**tiness of MSVC++. That's
it's not ANSI compliant! In quite a lot of the methods, there's a
multiple use of the same loop variable, as in
MyClass::MyMethod() {
for (Int_t i = 0; i < 10; i++)
cout << "In first loop " << i << endl;
for (Int_t i = 0; i < 10; i++)
cout << "In second loop " << i << endl;
}
Even though this is plain ANSI (i has only scope of the each loop),
MSVC++ claims this is a multiple defintion of i (just how stupid is
that). To compile, using MSVC++, one has to do:
MyClass::MyMethod() {
In_t i;
for (i = 0; i < 10; i++)
cout << "In first loop " << i << endl;
for (i = 0; i < 10; i++)
cout << "In second loop " << i << endl;
}
or use different variable names! Please keep this in mind when
writting code that needs to be portable to Windoze (why one would
want to do that ************************* (blacked out)). This
doesn't only apply to the MSVC++ compiler; in fact KCC behaves the
same way, as well as some other commercial compilers. It's
interresting to observe in this context that the GCC is the most ANSI
compliant compiler on the market (and rumour goes that it creates the
fastest code)!
Other changes where in the lines of changing
#include <unistd.h> // UNIX C header
to
#include <stdlib.h> // ANSI C header
As to style in general:
-----------------------
1: Data member and variable names should follow the ROOT conventions:
'f' first character of a mutable member
'fg' first two characters of static members
'k' first character of a constant member
'g' first character of global varible
2: Global variables is discouraged.
3: In any method that needs to get an array member, you better check
the index isn't out of range (otherwise you'll end up with a lot of
SIGSEGV).
4: Please don't use variable sized arrays as data members, but rather
pointers . That is, instead of
Float_t fArray[fSize]; //
Do
Float_t* fArray; //[fSize]
and then somewhere (in constructor!), do
fArray = new Float_t[fSize];
5: Use Double_t instead of Float_t for numerical computations.
6: Use the smallest possible data type for storage on disk.
7: Use TString instead of std:string or char arrays.
8: Use TMath::<Func> instead of std:<func>. E.g., instead of
y = sin(x);
do
y = TMAth::Sin(x);
9: When ever possible, do not use literal values for constants. E.g.,
instead of writting 3.14159265 for PI, use TMath::Pi().
10: Loop limits, array sizes, etc. should never be literal values, but
rather be set by (constant or static) members.
11: Header files <class>.h should conform to the following skeleton:
// -*- mode: c++ -*- For Emacs
//
// $Id$
// $Author$
// $Date$
//
#infdef BRAT_<class>
#define BRAT_<class>
#ifndef ROOT_TObject // And similar for any header file needed
#include "TObject.h" // in the declaration of the class. Please
#endif // don't use forward declarations.
class <class> <inheritance list>
{
<declaration body>
ClassDef(<class>,<version)
}
;
#endif
//
// $Log$
//
12: Implementation <class>.cxx files should conform tot he following
skeleton:
//
// $Id$
// $Author$
// $Date$
//
//____________________________________________________________________
//
// <Class description>
//
#ifndef BRAT_<class> // And similar for any header file needed
#include "<class>.h" // in the implmentation of the class.
#endif
//____________________________________________________________________
ClassImp(<class>);
//____________________________________________________________________
<class>::<method>(<arg list>)
{
// <method documantation>
}
//____________________________________________________________________
<and so on>
//
// $Log$
//
13: As for the units in BRAT, let me recap what Flemming has stated
earlier:
Quantity Unit
-----------------------
Length cm
Time ns
Energy GeV
Momentum GeV/c
Mass GeV/c^2
And I guess that
Field strength kGaus
An alternative is to use BrUnits when quoting physical
quantaties e.g., 1000 * BrUnits::MeV == 1 * BrUnits::GeV
As to the mult directory:
-------------------------
The code in this directory needs a lot of tidying up. One really grim
example was the assigment of a member in the header file!
Also, in BrRdoModuleMult.h, there's alot of lines like:
Int_t fTileParam[kNumTilesChanMax];
This is not right. Please do something like:
Int_t* fTileParam; //[kNumTilesChanMax];
and then in the constructor
fTileParam = new Int_t[kNumTilesChanMax];
and in the destructor
delete [] fTileParam;
Then there's something like:
Int_t siOccupation[6][42];
This should probably be coded like
const int kRows;
const int kSiColumns;
const int kTileColumns;
Short_t* fSiOccupation; //[kNumSiChanMax]
And the then in the constructor, one can do something like
BrRdoModuleMult::BrRdoModuleMult(int)
: kRows(6), kSiColumns(42), kTileColumns(8) {
fSiOccupation = new Short[kNumSiChanMax];
// Set row = i, column j of Si active
fSiOccupation[i * kRows + j] = 1; // i = 0,.., 5; j = 0,...,41;
}
I've implemented these changes; since there was no $Id$ string, I
couldn't bug anyone (sig). Also, who ever wrotw that class in the
first place: You don't need a ';' after a for/while/if loops closing
brace '}'! And please observe the general naming scheme and still in
BRAT (members begin with 'f' for one).
BrRdoModuleMult contains a TNtuple and a TFile. That is very poor
practice. Rather, the class should write to some branch in the output
BrEventNode. No output files should ever be opened by any RDO module,
other then a designated one, since creating files behind the back of
the user only promotes confusion and chaos.
The TNtuple in BrRdoModuleMult is horrific! It has something like 320
columns! If something like an Ntuple is really needed, one should use
a TTree (perhaps passed down from special module) and write on some
branch of that TTree, via a special data class (similar to BrSiDig,
etc.)
Hito: In BrDigitzeMultTile::Init there's a loop over GeantFBConvTable
with hardcoded limits, please investigate and correct!
Yours,
Christian -----------------------------------------------------------
Holm Christensen Phone: (+45) 35 35 96 91
Sankt Hansgade 23, 1. th. Office: (+45) 353 25 305
DK-2200 Copenhagen N Web: www.nbi.dk/~cholm
Denmark Email: cholm@nbi.dk
This archive was generated by hypermail 2b29 : Fri Sep 22 2000 - 17:36:28 EDT