Re: name for histograms

From: OKUMURA, Akira <oxon_at_icrr.u-tokyo.ac.jp>
Date: Thu, 15 Jun 2006 18:56:37 +0900


Hello Axel,

Thank you very much for your reply.
Could you give me one more advise for setters ?

Which setter do you like from your experience ?

void MyClass::SetHist(TH1* newhist)
{
hist = newhist; // 1) copy only the pointer // hist = newhist->Clone(); // 2) or use Clone() // hist = newhist->Clone(Form("copy of %s", newhist->GetName())); // 3) or use Clone() with new name
// newhist->Copy(hist); // 4) or use Copy() }

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 - 11:56:52 MEST

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