TH2F return by value

Probably it’s a c++ problem, but i can’t understand where I’m wrong:


class myclass
{
public:
    TH2F* h;
    myclass()
	{
	    h = new TH2F("h", "h", 10, 0, 10, 10, 0, 10);
	};
    
    TH2F getHisto() { return *h; };
    
};

int main()
{
    myclass a;
    TAxis* axis = a.getHisto().GetXaxis();
    for (int i = 1; i != axis->GetNbins(); ++i)
    {
	const double x = axis->GetBinLowEdge(i);
	cout << "x: " << x << endl;
    }
}

I got random numbers like 5e-319.

Hi,

The way you wrote it always forces a call to the TH2F copy constructor, creating a new object each time.
Try:

TH2F &getHisto() { return *h; };

or work with pointers:

TH2F *getHistoP() { return h; };

G. Ganis

[quote=“ganis”]Hi,

The way you wrote it always forces a call to the TH2F copy constructor, creating a new object each time.
Try:

TH2F &getHisto() { return *h; };

or work with pointers:

TH2F *getHistoP() { return h; };

G. Ganis[/quote]

I want to return copies because I want that the object will be valid also after deleting myclass (in the real code ~myclass delete h).

Hi,

From a reference or a pointer you can always do a Clone(), so saving your histogram.

Anyhow, your original code returns a valid copies of the histogram.
The problem is the way you use it in your test program. When you do

a.getHisto().GetXaxis();

the first function call (a.getHisto()) creates the context where to evaluate the second (GetAxis()); when this is done the context is deleted, together with the copy of the histogram. So the TAxis pointer you get back points to a non valid object.
You should save the histogram before using it:

TH2F h = a.getHisto();
TAxis* axis = h.GetXaxis();

or do not resolve the intermediate TAxis*, i.e

    for (int i = 1; i != a.getHisto().GetXaxis()->GetNbins(); ++i)
    {
   const double x = a.getHisto().GetXaxis()->GetBinLowEdge(i);
   cout << "x: " << x << endl;
    }

but this would be super-inefficient, creating a copy of the histogram for each call.

G. Ganis

[quote=“ganis”]Hi,

From a reference or a pointer you can always do a Clone(), so saving your histogram.

Anyhow, your original code returns a valid copies of the histogram.
The problem is the way you use it in your test program. When you do

a.getHisto().GetXaxis();

the first function call (a.getHisto()) creates the context where to evaluate the second (GetAxis()); when this is done the context is deleted, together with the copy of the histogram. So the TAxis pointer you get back points to a non valid object.
You should save the histogram before using it:

TH2F h = a.getHisto();
TAxis* axis = h.GetXaxis();

or do not resolve the intermediate TAxis*, i.e

    for (int i = 1; i != a.getHisto().GetXaxis()->GetNbins(); ++i)
    {
   const double x = a.getHisto().GetXaxis()->GetBinLowEdge(i);
   cout << "x: " << x << endl;
    }

but this would be super-inefficient, creating a copy of the histogram for each call.

G. Ganis[/quote]

I understand, but I don’t understand why my first version didn’t work.

So you did not understand … :wink:

Scope-wise, the sequence ‘a.getHisto().GetXaxis()’ can be represented like this

  a {
      getHisto {
           TH2F hc = <copy of a.h>
           hc {
                GetXaxis {
                     return fXaxis; // hc.fXaxis 
                }
           }
      }  // here hc is deleted, together with its fXaxis
  }

so what you get in the end (hc.fAxis) does not exists anymore, it is an invalid pointer.

Hope this time is more clear.
If you run it in the debugger you can check the execution flow.

G. Ganis

[quote=“ganis”][quote]
I understand, but I don’t understand why my first version didn’t work.
[/quote]
So you did not understand … :wink:

Scope-wise, the sequence ‘a.getHisto().GetXaxis()’ can be represented like this

  a {
      getHisto {
           TH2F hc = <copy of a.h>
           hc {
                GetXaxis {
                     return fXaxis; // hc.fXaxis 
                }
           }
      }  // here hc is deleted, together with its fXaxis
  }

so what you get in the end (hc.fAxis) does not exists anymore, it is an invalid pointer.

Hope this time is more clear.
If you run it in the debugger you can check the execution flow.

G. Ganis[/quote]

Ok, perfect, sorry for my ignorance…