Use "extern QStandardItemModel*" to provide access from anywhere?

[homerun4711]

Hello!

I would like to use some models (customers, articles, invoices,..) throughout my application, in every class. So far I passed them as parameters, but this gets to complicated. Is it common in this case to make models,... accessible via the keyword "extern"? Or is there another fine solution?

Kind regards,
HomeR

[SixDegrees]

Some will tell you to use a singleton pattern. This will work, but it's surprisingly difficult to get right. And in many cases it is serious overkill; it behaves exactly like any global variable, with all the potential pitfalls those bring, with the addition of lots of overhead to maintain.

As long as modifications to the global data are well controlled - it's usually best to arrange things so there are very few points where the data is written to, while reads can occur freely - plain old globals declared 'extern' are just fine, and more complex solutions offer no real advantages.

[squidge]

I'd go for more of a container solution. Rather than having the customers, articles, invoices, etc polluting the global namespace, create a management class for them and then pass a reference to this management class in the constructor of your other classes so they may use it to access the data they need.

I certainly wouldn't use global variables for the task. You start with a single global, and then it multiplies, and then before you know it you have code that isn't maintainable or reusable easily in other projects (you have to have knowledge of the application as a whole, rather than just specific classes)

[homerun4711]

Thanks for the anwers. I think I go for that container solution.

Is there some example code around for dealing with models in that way?

[squidge]

Something like: [pseudo code]

class DataManager
{
public:
DataManager(); // Remember to set m_customerModel etc to NULL in constructor
~DataManager();
void SetCustomerModel(QStandardItemModel *customerModel);
void SetInvoiceModel(QStandardItemModel *invoiceModel);
#ifdef DEBUG
QStandardItemModel *customerModel()
{
if (m_customerModel == NULL) throw E_NULLPOINTEREXCEPTION;
return m_customerModel;
}
#else
inline QStandardItemModel *customerModel() {return m_customerModel};
inline QStandardItemModel *invoiceModel() {return m_invoiceModel};
#endif
private:
QStandardItemModel *m_customerModel;
QStandardItemModel *m_invoiceModel;
};
 
DataManager *dmgr = new DataManager();
dmgr->SetCustomerModel (myModel);
[...]
myNewClass = new someclass(dmgr);
[...]
dmgr->customerModel()->doSomething(); or pass dmgr->customerModel() pointer to some other function or class. 
If you added another pointer to the class and forget to call the set(), it'll just throw an exception to remind you.

class DataManager
{
public:
DataManager(); // Remember to set m_customerModel etc to NULL in constructor
~DataManager();
void SetCustomerModel(QStandardItemModel *customerModel);
void SetInvoiceModel(QStandardItemModel *invoiceModel);
#ifdef DEBUG
QStandardItemModel *customerModel()
{
if (m_customerModel == NULL) throw E_NULLPOINTEREXCEPTION;
return m_customerModel;
}
#else
inline QStandardItemModel *customerModel() {return m_customerModel};
inline QStandardItemModel *invoiceModel() {return m_invoiceModel};
#endif
private:
QStandardItemModel *m_customerModel;
QStandardItemModel *m_invoiceModel;
};

DataManager *dmgr = new DataManager();
dmgr->SetCustomerModel (myModel);
[...]
myNewClass = new someclass(dmgr);
[...]
dmgr->customerModel()->doSomething(); or pass dmgr->customerModel() pointer to some other function or class. 
If you added another pointer to the class and forget to call the set(), it'll just throw an exception to remind you.

To copy to clipboard, switch view to plain text mode 

[homerun4711]

Thank you very much. I already wrote some lines on this also called my class DataManager

Kind regards,
HomeR

[nroberts]

Some will tell you to use a singleton pattern. This will work, but it's surprisingly difficult to get right. And in many cases it is serious overkill; it behaves exactly like any global variable, with all the potential pitfalls those bring, with the addition of lots of overhead to maintain.

This is simply not the case.

As long as modifications to the global data are well controlled

And THAT would be exactly the difference (one of the most important anyway). It's impossible to control access to a global.

[homerun4711]

I wrote a container class, but now the app is crashing. I guess there is a pointer
error somewhere, but I can't find it.

Last output from debug is

DataManager::customerTableView()

Ok, first call made from here:

void MainWindow::showcustomerlist()
{
    AddressBook *customerlist = new AddressBook(dmgr);
}

void MainWindow::showcustomerlist()
{
    AddressBook *customerlist = new AddressBook(dmgr);
}

To copy to clipboard, switch view to plain text mode 

AddressBoook.cpp

AddressBook::AddressBook(DataManager* dmgr, QWidget *parent): QDialog(parent)
{
    setupUi(this);
    tableView = dmgr->customerTableView();
}

AddressBook::AddressBook(DataManager* dmgr, QWidget *parent): QDialog(parent)
{
    setupUi(this);
    tableView = dmgr->customerTableView();
}

To copy to clipboard, switch view to plain text mode 

DataManager.h

QStandardItemModel *model_customer;
QTableView *m_customerTableView;
QTableView *customerTableView();

QStandardItemModel *model_customer;
QTableView *m_customerTableView;
QTableView *customerTableView();

To copy to clipboard, switch view to plain text mode 

DataManager.cpp

model_customer = new QStandardItemModel;
m_customerTableView = new QTableView;
m_customerTableView->setModel(model_customer);
QTableView *DataManager::customerTableView() {return m_customerTableView;}

model_customer = new QStandardItemModel;
m_customerTableView = new QTableView;
m_customerTableView->setModel(model_customer);
QTableView *DataManager::customerTableView() {return m_customerTableView;}

To copy to clipboard, switch view to plain text mode 

I hope this is enough code, just tell my if I missed some.

Kind regards,
HomeR

[SixDegrees]

This is simply not the case.

Actually, it is. I've encountered dozens of singleton implementations, and have yet to see a single one that was properly implemented. In several cases, poorly written singletons have been the source of major bugs that proved very difficult to track down. Finding and fixing the errors was made even more difficult by blind acceptance of the notion that singletons are required and somehow superior to simple globals, but programmer arrogance and the closed mindset it brings to the table is a seperate issue.

And THAT would be exactly the difference (one of the most important anyway). It's impossible to control access to a global.

Contrary to popular belief, programmers are not mindless automata; many, or at least some, are capable of reading documentation and can even understand it. If I use a global variable, I treat it differently than I do a local because I typically bring my brain along with me when coding. Declaring an outright ban on globals is mindless oversimplification; there are, in fact, cases where they are both appropriate and preferred.

More thoughts on singletons: Singletons Are Evil.

[squidge]

I wrote a container class, but now the app is crashing. I guess there is a pointer
error somewhere, but I can't find it.

Do you set all the internal variables to NULL in the constructor for the container class?

For the debug version of the application, do you check for NULL before returning a pointer as in my example code?

Is 'dmgr' created using 'new' rather than being stored on the stack?

[homerun4711]

Hm, no, I do not set all internal variables to NULL in the constructor.

Is this neccessary even if I initialize them? If yes,, why?

Constructor is

DataManager::DataManager()
{
...
    setupTableViews();
...
}

DataManager::DataManager()
{
...
    setupTableViews();
...
}

To copy to clipboard, switch view to plain text mode 

void DataManager::setupTableViews()
{
    m_customerTableView = new QTableView;
}

void DataManager::setupTableViews()
{
    m_customerTableView = new QTableView;
}

To copy to clipboard, switch view to plain text mode 

You other question on dmgr:

MainWindow::MainWindow(QWidget *parent)
    : QMainWindow(parent)
{
...  
  DataManager* dmgr = new DataManager();
}

MainWindow::MainWindow(QWidget *parent)
    : QMainWindow(parent)
{
...  
  DataManager* dmgr = new DataManager();
}

To copy to clipboard, switch view to plain text mode 

[squidge]

Fair enough, if you are setting the variables directly in the datamanager, then you don't need to set them to null or check the validity of them, I was assuming that another class would be initialising them with valid values.

[homerun4711]

Ok. I am still searching for the error...

Is this valid?

private:
    QStandardItemModel* model;

private:
    QStandardItemModel* model;

To copy to clipboard, switch view to plain text mode 

and in the constructor

AddressNew::AddressNew(DataManager* dmgr, QWidget *parent)
    : QDialog(parent)
{    
    model = dmgr->customerModel();
}

AddressNew::AddressNew(DataManager* dmgr, QWidget *parent)
    : QDialog(parent)
{    
    model = dmgr->customerModel();
}

To copy to clipboard, switch view to plain text mode 

Or must there be a

model = new QStandardItemModel;

model = new QStandardItemModel;

To copy to clipboard, switch view to plain text mode 

before the

model = dmgr->customerModel();

model = dmgr->customerModel();

To copy to clipboard, switch view to plain text mode 

?

[tbscope]

Is this valid?

Syntax: yes
Technically: this is a little bit "iffy"

First problem: are you sure that dmgr is always valid? If dmgr isn't valid at some point, your code will most likely crash.
Second problem: are you sure that customerModel() always returns a valid pointer? Again, this might crash your program.

You don't need to create a new standard item model first. The dmgr object will provide the one you want.
However, I do not see any safety checks or any guarantees that the model object is valid and existing.

[pkohut]

Ok. I am still searching for the error...

AddressNew::AddressNew(DataManager* dmgr, QWidget *parent)
    : QDialog(parent)
{    
    model = dmgr->customerModel();
}

AddressNew::AddressNew(DataManager* dmgr, QWidget *parent)
    : QDialog(parent)
{    
    model = dmgr->customerModel();
}

To copy to clipboard, switch view to plain text mode 

Wondering the same as tbscope, is dmgr valid. Some defensive coding will help solve these types of errors.

AddressNew::AddressNew(DataManager* dmgr, QWidget *parent)
    : QDialog(parent)
{
    Q_ASSERT(dmgr);
    model = dmgr->customerModel();
    Q_ASSERT(model);
}

AddressNew::AddressNew(DataManager* dmgr, QWidget *parent)
    : QDialog(parent)
{
    Q_ASSERT(dmgr);
    model = dmgr->customerModel();
    Q_ASSERT(model);
}

To copy to clipboard, switch view to plain text mode 

If either dmgr or model is NULL then the program will stop and a message will be printed to the debug console.

Unless you know for fact that a function will always return a valid pointer, the the pointer should be checked, either with if statements or asserts.

[nroberts]

Actually, it is. I've encountered dozens of singleton implementations, and have yet to see a single one that was properly implemented. In several cases, poorly written singletons have been the source of major bugs that proved very difficult to track down. Finding and fixing the errors was made even more difficult by blind acceptance of the notion that singletons are required and somehow superior to simple globals, but programmer arrogance and the closed mindset it brings to the table is a seperate issue.

LOL! Well, I guess if YOU'VE never seen a "properly implemented" (and I guess we'll have to guess what you mean by that) singleton then they MUST be exactly the same as globals.

As you sit there lamenting the poor implementations you've seen of Singletons, perhaps you should spend a little time considering exactly what criterion you'd even use to critique an implementation of a global variable. Personally, since a variable with global scope is a variable with global scope, nothing more or less, I can't think of ANY criterion that could be used. I suppose we could begin asking the same questions one might ask about a singleton, such as:

Is it correctly initialized?

- Can't tell without looking at entire source tree.

Is it correctly protected in a multithreaded environment?

- It's not protected at all so, no.

And no, this isn't the same as asking, "Was everyone who's ever worked on our product always having a good brain day when they did anything regarding this variable?"

Contrary to popular belief, programmers are not mindless automata; many, or at least some, are capable of reading documentation and can even understand it. If I use a global variable, I treat it differently than I do a local because I typically bring my brain along with me when coding. Declaring an outright ban on globals is mindless oversimplification; there are, in fact, cases where they are both appropriate and preferred.

Two things:

1) Never said globals can't be used. I would say a singleton is generally better if for no other reason than organization, but I never declared any ban on globals.

2) Your very same argument speaks against every single OO construct out there. _I'M_ smart enough. _I_ bring my brain. _I_ know what I'm doing. _I_ don't use RAII because _I_ bring my brain when using pointers.

Of course, nothing you just said even responds, much less refutes what I said. Singletons provide access control and that's simply not possible with globals. If you want to argue whether access control is worthwhile because YOU are too smart to use it...be my guest. I'm a pretty darn smart guy and I'll admit that I'm not THAT smart....the brain power required for such an exercise very quickly exceeds my ability to track and I don't even want to THINK about what the interns might do.

Furthermore, as you quite fully admit in the very statements you use, some or many programmers are NOT as smart as you. If you can't write code that they can maintain...well, you're not going to be of any use to me even if you really ARE that smart.

[pkohut]

Actually, it is. I've encountered dozens of singleton implementations, and have yet to see a single one that was properly implemented.

Including QApplication and QCoreApplication?

[homerun4711]

Huh, I don't even know what singletons are...

Ok, defensive coding sounds good

I tried to find the error, but still no success.

How is it possible to get the nice messages where the error happens?
I forced a NULL pointer to try it...

//model_customer = new QStandardItemModel;
    ...then
    Q_ASSERT(model_customer);
    ...or
    Q_CHECK_PTR(model_customer);

//model_customer = new QStandardItemModel;
    ...then
    Q_ASSERT(model_customer);
    ...or
    Q_CHECK_PTR(model_customer);

To copy to clipboard, switch view to plain text mode 

But neither Q_ASSERT nor Q_CHECK_PTR produce an output like

ASSERT: "b == 0" in file div.cpp, line 7

Instead I get

&"warning: GDB: Failed to set controlling terminal: Invalid argument\n"
terminate called after throwing an instance of 'std::bad_alloc'
what(): std::bad_alloc

Back to the original problem:

addresnew.h

QStandardItemModel* model;

QStandardItemModel* model;

To copy to clipboard, switch view to plain text mode 

The following call for model = dmgr->customerModel(); crashes.
Disabling it and just output some text works.

addressnew.cpp

AddressNew::AddressNew(DataManager* dmgr, QWidget *parent)
    : QDialog(parent)
{
 
    Q_CHECK_PTR(dmgr);
    Q_ASSERT(dmgr);
 
    // the following function is accessible
    //model = dmgr->customerModel(); //should usually assign QStandardModelItem*, crashing!
    dmgr->customerModel(); //changed to test, just write QTextStream stdout. working!
}

AddressNew::AddressNew(DataManager* dmgr, QWidget *parent)
    : QDialog(parent)
{

    Q_CHECK_PTR(dmgr);
    Q_ASSERT(dmgr);

    // the following function is accessible
    //model = dmgr->customerModel(); //should usually assign QStandardModelItem*, crashing!
    dmgr->customerModel(); //changed to test, just write QTextStream stdout. working!
}

To copy to clipboard, switch view to plain text mode 

datamanager.h

QStandardItemModel *model_customer;
QStandardItemModel *customerModel();
void setupModels();

QStandardItemModel *model_customer;
QStandardItemModel *customerModel();
void setupModels();

To copy to clipboard, switch view to plain text mode 

datamanager.cpp

void DataManager::setupModels()
{
 
    QTextStream out(stdout);
    out << "setupModels()\n";
 
    model_customer = new QStandardItemModel;
    Q_ASSERT(model_customer);
    //not crashing here
}
 
QStandardItemModel *DataManager::customerModel()
{
   //QTextStream out(stdout);
   //out << "set customerModel()\n";
   Q_ASSERT(model_customer);
   return model_customer; //crash here if returning model is enabled
}

void DataManager::setupModels()
{

    QTextStream out(stdout);
    out << "setupModels()\n";

    model_customer = new QStandardItemModel;
    Q_ASSERT(model_customer);
    //not crashing here
}

QStandardItemModel *DataManager::customerModel()
{
   //QTextStream out(stdout);
   //out << "set customerModel()\n";
   Q_ASSERT(model_customer);
   return model_customer; //crash here if returning model is enabled
}

To copy to clipboard, switch view to plain text mode 

I have no idea what causes the problem, can you help me?
Just tell if you need more code.

[wysota]

Huh, I don't even know what singletons are...

You can read about them for example here: [wiki]Singleton pattern[/wiki]

[pkohut]

It would appear that model_customer is not initialized when customerModel() is called. Was DataManager::setupModels() called before trying to make a new instance of AddressNew? And, do you initialize all member pointers in their constructor's?

DataManager::DataManager()
{
    model_customer = NULL;
}

DataManager::DataManager()
{
    model_customer = NULL;
}

To copy to clipboard, switch view to plain text mode 

If not then they are uninitialized and point anywhere.

As for the gdb warning, no idea (running VS).