c++ Use a class variable in a non-class function

Question

I'm facing this novice issue. Assume the class MainFrame (the following code isn't going to compile - I'm trying to give a basic idea of what I'm doing because I think my problem is easy to solve by someone more knowledgeable than me) which lives on file gui.cxx along with other functions. Note that this is part of a larger project so I'm skipping the main.cxx which I have included gui.h.

In the function start_gui_with_config() I'm trying to use an object from MainFrame. At the moment is declared as private so I'm expecting to have an text_data_path was not declared in this scope.

I also declared this variable as public and static in the class definition in gui.h but then I get the same error when using either text_data_path ->SetText(data_path);.

When I'm using MainFrame::text_data_path ->SetText(data_path); (still text_data_path is declared as private and static) I get the error undefined reference to MainFrame::text_data_path in any line I'm using text_data_path within the MainFrame::MainFrame constructor (file gui.cxx) and strangely I get this error twice for each line.

Finally I tried making all the functions (start_gui(), start_gui_with_config()) part of MainFrame and I declared them as either static void (in this case I got an error error: cannot declare member function static void MainFrame::start_gui_with_config() to have static linkage on the gui.cxx ) or void (in this case I got the error error: cannot call member function void MainFrame::start_gui_with_config() without object on the main.cxx).

Any idea on how to use text_data_path in a function (i.e. start_gui_with_config()) that doesn't belong to the class?

gui.cxx

#include "../include/gui.h"    

MainFrame::MainFrame(const TGWindow *p, UInt_t width, UInt_t height):TGMainFrame(p, width, height, kMainFrame|kHorizontalFrame){

// Define widgets
text_data_path = new TGTextEntry("/data/2020");

}

//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
// This is a virtual constructor
//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
MainFrame::~MainFrame() {
   // Clean up used widgets: frames, buttons, layout hints
   Cleanup();
}//_____MainFrame::~MainFrame()

//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
// This is to start the GUI with default settings
//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
void start_gui(){
   // Popup the gui
   std::cout << "Starting the gui" << std::endl;
   new MainFrame(gClient->GetRoot(), 1000, 800);
}//_____start_gui()

//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
// This is to start the GUI using the configuration file from previous session
//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
void start_gui_with_config(){

    TString data_path = gSystem->GetFromPipe("awk '{if(NR==1) print $NF}' Config/last_session.cfg.viewer");
    
    start_gui();
    
    MainFrame::text_data_path->SetText(data_path);
    
}//____MainFrame::start_gui_with_config()

gui.h

#ifndef ___GUI_H
#define ___GUI_H

//ROOT Includes
#include <TGTextEntry.h>

//C++ includes
using namespace std;

class MainFrame : public TGMainFrame {
private:
    // Widgets
    TGTextEntry         *text_data_path;
    
public:

    // Widgets
    //static TGTextEntry         *text_data_path;

   MainFrame(const TGWindow *p, UInt_t width, UInt_t height);
   virtual ~MainFrame();

   //void start_gui_with_config();
   //static void start_gui();

ClassDef (MainFrame,0);// Remove for ROOT6 and rootcling
};

void start_gui();
void start_gui_with_config();

#endif

`new MainFrame(gClient->GetRoot(), 1000, 800);` you probably don't want to throw away the pointer you created here. — drescherjm, Jul 02 '20 at 17:27
The first thing you need to decide is whether you want to make `text_data_path` static or not. This isn't something you decide on the basis of making your code compile or not. It's a design decision you make on the basis of what that variable means for the `MainFrame` class. Once you've decided that we can them talk about the right way to make your code compile. — john, Jul 02 '20 at 17:27
// This is a virtual constructor, no such thing, it's a destructor — QuentinUK, Jul 02 '20 at 17:32
There can be more than one MainFrame object. Which one's text_data_path do you want to get? — user253751, Jul 02 '20 at 17:46
Unrelated: It is illegal to use two underscores in a row in any identifier unless you're writing a compiler or a Standard Library implementation. Details on that here: [What are the rules about using an underscore in a C++ identifier?](https://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier) — user4581301, Jul 02 '20 at 17:48
@drescherjm Thanks for your comment! How can I do that? Why are you saying that? I haven't realised that I'm throwing it away. — Thanos, Jul 02 '20 at 18:01
@john Thanks for that. Honestly I have no idea if I want it to be static or not. I'm quite novice so I can't think why would I or wouldn't want something `static`. From what I know so far, `static` is used to have the function/object/variable available outside of the frame where it's created. I'm sure that I miss a lot of knowledge, but I can't think of anything else. — Thanos, Jul 02 '20 at 18:04
@user253751The one that it's created in the `start_gui()`. It's also the only one when running the program. — Thanos, Jul 02 '20 at 18:06
`new MainFrame(gClient->GetRoot(), 1000, 800);` returns a pointer to the object you just created. To access the MainFrame object you just created you need to use the returned pointer however instead of saving this in a variable you throw it away. — drescherjm, Jul 02 '20 at 18:10
@drescherjm You are referring to using that in `start_gui()`. I'm not sure if I understand what I have to do. I'm using this thing, because it was an example in ROOT6 so I thought it was correct, although it seems bizarre. — Thanos, Jul 02 '20 at 18:13
Maybe you want to change `void start_gui() { ....}` to `MainFrame* start_gui() { return new MainFrame(gClient->GetRoot(), 1000, 800); }` — drescherjm, Jul 02 '20 at 18:21
@Thanos Not quite, static (in this context) means that the variable exists independently of any instance of `MainFrame`, so if `text_data_path` is static it exists even if you've never created a `MainFrame` object at all, and even if you've created 20 instances of `MainFrame` there's only one instance of` `text_data_path`. On the other hand a non-static variable is created with each and every instance of it's containing class. So you have to decide which of those two models is more appropriate in this case. — john, Jul 02 '20 at 18:25
@john Thanks for the insight! In this case, it makes no sense to have `text_data_path` if the `MainFrame` doesn't exist. So I guess `static` is not needed, right? — Thanos, Jul 02 '20 at 18:43
@drescherjm I did that but still I can't get it to compile. Or maybe you were pointing out a more correct way to code that. — Thanos, Jul 02 '20 at 18:47
@Thanos That seems reasonable. For a non-static `text_data_path` it seems that drescherjm is pointing you on the correct path. To access that variable the first thing you need is a pointer to your `MainFrame` object. — john, Jul 02 '20 at 18:47
@john Thanks a lot for your encouragement! So I declared `text_data_path` as `private`, changed the `start_gui()` function `MainFrame* start_gui(){return new MainFrame(gClient->GetRoot(), 1000, 800);}` and then the `start_gui_with configuation` is `void start_gui_with_configuration(){ ... start_gui(); text_data_path ->SetText(data_path);}` but I still get `text_data_path was not declared in this scope` — Thanos, Jul 02 '20 at 18:59
@john: I think I solved it but I don't know if it's elegant. I declared `text_data_path` as public, in `start_gui_with_config()` the frame is declared as `MainFrame *frame = new MainFrame(gClient->GetRoot(), 1000, 800);` and then when calling `text_data_path` I just use `frame->text_data_path->SetText(data_path);`. Although it works, I'm wondering if it's elegant and efficient memory/code-wise. — Thanos, Jul 02 '20 at 19:15
There's no efficiency problem there. However having public data members is normally considered bad from a style perspective. But that's a whole different topic. — john, Jul 03 '20 at 05:00

score 2 · Answer 1 · answered Jul 02 '20 at 17:29

2

I suggest you use a setter on the MainFrame class:

void setDatapathText(TString const& newDatapath) { 
     text_data_path->SetText(data_path);
}

You can then call it like so in your start_gui_with_config function :

auto frame = MainFrame(p, w, h);
frame.setDatapathText(data_path);

Be careful, your code clearly has memory management problems, and as a general rule you should never deal with raw new and delete outside of smart pointers. I suggest making sure you are comfortable with dynamic allocation, else I'm afraid you will be facing hard-to-debug errors earlier than expected

answered Jul 02 '20 at 17:29

Magix

4,989
7
26
50

1

You've declared `frame` as a `MainFrame` object (not a pointer to a `MainFrame`, so it will be destroyed when the function returns (which is not the intended behavior). – 1201ProgramAlarm Jul 02 '20 at 17:36
Yes. You need to store it somewhere, but creating it on the stack doesn't mean you are forced to lose it. You can copy it (or `std::move()` it) wherever you want, but only you can know where (a global, a static variable, an object member, or wherever else). It would have been the same if it had been created as a pointer – Magix Jul 02 '20 at 17:40
@Magix Thanks a lot for your answer! Why do you say I have memory management problems? I'm a beginner so any advice is more than welcome! – Thanos Jul 02 '20 at 18:08
@Magix I tried your solution, but I get the same thing which to me it was expected. `text_data_path’ was not declared in this scope` on the `setDatapathText` function. – Thanos Jul 02 '20 at 18:52

drescherjm · Accepted Answer · 2020-07-02T19:53:45.433

Your problem is you throw away the pointer to your MainFrame so you have no way to access it in start_gui_with_config() after you created the MainFrame.

One way to fix this is to change the signature of void start_gui(); to MainFrame* start_gui(); in your gui.h header.

In the gui.cxx change the implementation to

MainFrame* start_gui() {
   return new MainFrame(gClient->GetRoot(), 1000, 800); 
}

And then in your start_gui_with_config() use the pointer like this:

void start_gui_with_config(){

    TString data_path = gSystem->GetFromPipe("awk '{if(NR==1) print $NF}' Config/last_session.cfg.viewer");
    
    MainFrame* frame = start_gui();
    frame->text_data_path->SetText(data_path);
    
}//____MainFrame::start_gui_with_config()

This code assumes the MainFrame object destroys itself otherwise the code will leak memory. I assume this destruction happens after the window closes. I have seen other GUI frameworks like Qt do this.

c++ Use a class variable in a non-class function

2 Answers2