Home » CBM » CBMROOT » General » Effective C++
| Effective C++ [message #13067] |
Fri, 17 February 2012 09:09  |
Volker Friese
Messages: 365
Registered: April 2004
Location: GSI CBM
|
first-grade participant |
From: *gsi.de
|
|
We want to remove, throughout the repository, all compiler warnings related to the warning level -Weffc++. These mostly concern member intialisation and assignment/copy constructors for classes with pointer members (see CBM Software Meeting, 16 February 2012).
A SVN ticket (#40) was created for this project. Please assign all corresponding commits to this ticket.
|
|
|
|
|
|
| Effective C++: Member initialisation [message #13069 is a reply to message #13067] |
Fri, 17 February 2012 09:14  |
Volker Friese
Messages: 365
Registered: April 2004
Location: GSI CBM
|
first-grade participant |
From: *gsi.de
|
|
Hi
In the following I will explain how one get rid of one of the two most common warnings which show up when one switch on the effc++ warnings from gcc. The second warning is described in the
topic How to get rid of -Weffc++ warnings (part 2).
The first warning complaines about class members which are not initialized in the member initialization list. Please check example output below.
| Quote: |
warning: 'CbmFieldMap::fFileName' should be initialized in the member initialization list
|
Before we start with a real example from CBM code we have to do
some theory about C++ (taken from Scot Meyers Effective C++).
The problem is that one can't rely on that C++ will initialize Objects during declaration. C++ sometimes initialize the Objects during declaration, sometimes it doesn't do it.
In the above example x will be initialized (with 0) or not depending on the context. Due to this reason it is best practice to initialize the variables before usage. See example below.
int x = 0;
const char *text = "CBM is the best experiment ever."
When working with objects the initialization of all data members should be done in the constructor.
Coming back now to the CBM example. As example the class CbmFieldMap is taken.
The data members of this class are the following (taken from the header file)
TString fFileName;
Double_t fScale;
Double_t fPosX, fPosY, fPosZ;
Double_t fXmin, fXmax, fXstep;
Double_t fYmin, fYmax, fYstep;
Double_t fZmin, fZmax, fZstep;
Int_t fNx, fNy, fNz;
TArrayF* fBx;
TArrayF* fBy;
TArrayF* fBz;
Double_t fHa[2][2][2];
Double_t fHb[2][2];
Double_t fHc[2];
On of the constructors looks like
CbmFieldMap::CbmFieldMap()
{
fPosX = fPosY = fPosZ = 0.;
fXmin = fYmin = fZmin = 0.;
fXmax = fYmax = fZmax = 0.;
fXstep = fYstep = fZstep = 0.;
fNx = fNy = fNz = 0;
fScale = 1.;
fBx = fBy = fBz = NULL;
fPosX = fPosY = fPosZ = 0.;
fName = "";
fFileName = "";
fType = 1;
}
To all the data memebers values are assigned in the body of the constructor. To mention here is the fact that one assignes values to fPosX, fPosY and fPosZ two times which shouldn't hurt but is definetly not something which is intended. The other point
to mention is the assignment of values to members of the base class. This is necessary because there is no appropriate constructor of the base class.
The problem here is that the data members have in the end the expected values but it is not the best solution for the task.
The C++ rules define that alle data members should be initialized before the body of the constructor is entered. In the above example the values of the data members were not initialized but assigned. The initialization take place when the default constructors are called before entering the functions body. This is not true for all the integrated types (int, float ...), because here it is not guaranteed that the data members are initalized at all before entering the function body.
There is much more in the reference (Effective C++), but as a rule of thumb you should remeber to initialize all data members in in the member initialization list. The order of the data members should be the same as the order they are declared in the header file.
The corrected constructur now looks like the one below. Before initializing the data members of the class the default constructor of the base class is "called". Then the data members show up in the same order as they are declared in the header file. In the body of the constructor only the default values to all elements of the arrays are assigned. I don't know any way to initalize the elements of an array in the member initalization list.
Also here the values of data members of the base class are
assigned because there is no appropriate constructpr of the base class. If there would be such a constructor one could call this constructor in the member initilization list.
CbmFieldMap::CbmFieldMap()
: FairField(),
fFileName(""),
fScale(1.),
fPosX(0.),
fPosY(0.),
fPosZ(0.),
fXmin(0.),
fXmax(0.),
fXstep(0.),
fYmin(0.),
fYmax(0.),
fYstep(0.),
fZmin(0.),
fZmax(0.),
fZstep(0.),
fNx(0),
fNy(0),
fNz(0),
fBx(NULL),
fBy(NULL),
fBz(NULL)
{
// Initilization of arrays is to my knowledge not
// possible in member initalization lists
for (Int_t i=0; i < 2 ; i++) {
fHc[i]=0;
for (Int_t j=0; j < 2 ; j++) {
fHb[i][j]=0;
for (Int_t k=0; k < 2 ; k++) {
fHa[i][j][k]=0;
}
}
}
// Assign values to data members of base classes
// There is no appropriate constructor of the base
// class.
fName = "";
fType = 1;
}
I hope this topic helps to understand the warning and how to correct the code. Remarks and corrections are very welcome
Ciao
Florian
|
|
|
|
|
|
| If you really want the copy constructor [message #13071 is a reply to message #13070] |
Fri, 17 February 2012 09:42 |
Volker Friese
Messages: 365
Registered: April 2004
Location: GSI CBM
|
first-grade participant |
From: *gsi.de
|
|
The copy constructor which is created by default by the compiler if you do not do it yourself will just copy each data member by value. If your class just contains normal type members, this is usually want you want.
If your class, however, contains a pointer member, things are different, because the default copy constructor will not copy the object the pointer points to, but just the pointer value. So, the pointer member of the new object will point to the same location as that of the original object. If the original object is removed, then its destructor will delete the object its pointer member points to (if properly implemented). So, the pointer member of the new, copied object, will point to an invalid address. The best you can get is then a segmentation fault.
If for such a class you do not need the copy or assignment constructor, follow the instructions given in the mother posting. If you do need it, you have to implement it yourself. In most cases that means to manually copy the normal members one by one. In addition, for the pointer member you have to instantiate an object it points to, and use the copy constructor of this object to create it from the one of the original class.
Example:
class Example {
double firstMember;
FairMCPoint* pointerMember;
// Assignment operator
Example& operator = (Example const & source) {
firstMember = source.FirstMember;
pointerMember = new FairMCPoint(*(source.pointerMember));
}
// Copy constructor, using the assignment operator
Example(Example const & source) { *this = source; }
}
This is just an example, there is no general solution. You have to decide on your own what in your particular case the assignment operator and the copy constructor are supoosed to do.
|
|
|
|
Goto Forum:
Current Time: Tue Nov 26 00:00:28 CET 2024
Total time taken to generate the page: 0.01699 seconds
|
GSI Forum
Search
Help
Members
Login
Home
