Re: name for histograms

From: Axel Naumann <Axel.Naumann_at_cern.ch>
Date: Thu, 15 Jun 2006 12:37:03 +0200


Hi,

now we're getting into the "unofficial answer" section - this is my personal opinion:

> Which setter do you like from your experience ?
>
> void MyClass::SetHist(TH1* newhist)
> {
> hist = newhist; // 1) copy only the pointer

This is OK if you want to modify a histogram, and you want to make sure that the modified histogram is used later on. E.g. for a function

void TSinatra::SetHist(TH1* h) {

   fHist = h;
}

void TSinatra::DoItMyWay() {
  fHist->SetLineColor(kBLUE);
}

> // hist = newhist->Clone(); // 2) or use Clone()

I would not use it, as it doesn't set the new hist's name. Instead, use

void MyClass::SetHist(TH1* newhist) {
  if (hist) delete hist; // make sure old version is gone   hist = (TH1*) newhist->Clone(GetName()); }

This is my personal favorite for any manipulation on histograms. It makes sure that there are as little side effects as possible. And the new name is based on who "owns" the histogram (if the histogram will be deleted within MyClass later on), or at least on who created it (if the histogram is handed over to the user, and will not be deleted by MyClass).

> // newhist->Copy(hist); // 4) or use Copy()

Better use Clone, which does a complete copy. See http://root.cern.ch/root/html/TH1#TH1:Copy for the difference between the two.

As you said it all depends on what you're trying to do with the histogram. I hope these explanations are clear enough so you can pick the right option.

Cheers, Axel.

> Of course it depends on my application. But this is first time for me to
> make a library based on ROOT. Therefore I do not know which is the best
> for me.
>
> I think all of the above has demerit.
> 1) Users may delete newhist after using SetHist() without assignment NULL.
> 2) & 4) hist and newhist have same name.
> 3) New name always have 'copy of' or similar.
>
> Sincerely,
>
> OKUMURA, Akira oxon_at_icrr.u-tokyo.ac.jp
> Institute for Cosmic Ray Research, University of Tokyo
> 5-1-5 Kashiwanoha Kashiwa Chiba 277-8582 Japan
> Phone/Fax : +81 4-7136-3153
> Skype : okumura.akira
>
> On 2006/06/15, at 17:03, Axel Naumann wrote:
>

>> Hi,
>>
>> first of all, you should use pointers to histograms:
>>
>> class MyClass : public TObject {
>> private:
>>   TH1D *hist;
>> public:
>>   // base class's default constructor is called automatically
>>   MyClass() : hist(0) {}
>>   TH1* GetHist();
>> };
>>
>> // in the source file:
>> TH1* MyClass::GetHist() {
>>   if (!hist)
>>     hist = new TH1D("myhist", "my histogram;x;Entries", 100, 0., 1.);
>>   return hist;
>> }
>>
>>
>> Then you should try to give these histograms names which depend on the
>> object, i.e. which are different for each object. ROOT allows to lookup
>> histograms by their names - if they all have the same name that doesn't
>> work. So this version is better:
>>
>> class MyClass : public TNamed {
>> private:
>>   TH1D *hist;
>> public:
>>   MyClass() : hist(0) {}
>>   TH1* GetHist();
>> };
>>
>> // in the source file:
>> TH1* MyClass::GetHist() {
>>   if (!hist)
>>     hist = new TH1D(GetName(), GetTitle(), 100, 0., 1.);
>>   return hist;
>> }
>>
>> You should put GetHist() into the source because it has a call to "new";
>> as a generic rule no calls to new and delete should be in a header (due
>> to compiler flag specific, incompatible implementations of new and
>> delete, which might get mixed when mixing compiler flags).
>>
>> Cheers, Axel.
>>
>>
>> OKUMURA, Akira wrote:
>>> Hello ROOTers,
>>>
>>> If I make a class which has histogram as a member as below, at the line
>>> of declaration of 'MyClass b' ROOT warns 'Potential memory leak'.
>>>
>>> class MyClass : public TObject {
>>> private:
>>>   TH1D hist;
>>>   /* other members */
>>> public:
>>>   MyClass() : TObject() {hist = TH1D(0, 0, 1, 0, 1);}
>>>   /* other functions */
>>> };
>>>
>>> void test()
>>> {
>>>   MyClass a;
>>>   MyClass b; // => says "potential memory leak
>>> }
>>>
>>> I understand why it happens and that the solution which avoids this is
>>> to specify identical name for each histograms. For example, I can give
>>> them identical names through arguments for a constructor. However if
>>> MyClass has many histograms and they need to be initialize in the
>>> default constructor, the default constructor will become messy.
>>>
>>> MyClass(const char* name1, const char* name2, /*continue*/) : TObject()
>>> {
>>>   hist1 = TH1D(name1, name1, 1, 0, 1);
>>>   hist2 = TH1D(name2, name2, 1, 0, 1);
>>>   // continue
>>> }
>>>
>>> I would like to know how do you implement your application in this case.
>>> Is there any standard method ?
>>>
>>> Sincerely,
>>>
>>> OKUMURA, Akira oxon_at_icrr.u-tokyo.ac.jp
>>> Institute for Cosmic Ray Research, University of Tokyo
>>> 5-1-5 Kashiwanoha Kashiwa Chiba 277-8582 Japan
>>> Phone/Fax : +81 4-7136-3153
>>> Skype : okumura.akira
>>>
>>
>>

>
Received on Thu Jun 15 2006 - 12:37:14 MEST

This archive was generated by hypermail 2.2.0 : Mon Jan 01 2007 - 16:31:59 MET