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.

[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);
}

AddressBoook.cpp


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

DataManager.h


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

DataManager.cpp


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

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 (http://c2.com/cgi/wiki?SingletonsAreEvil).

[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();
...
}


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

You other question on dmgr:


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

[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;

and in the constructor


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

Or must there be a

model = new QStandardItemModel;
before the

model = dmgr->customerModel();
?

[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();
}


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);
}

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);


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;

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!
}

datamanager.h



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


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
}


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: Singleton pattern

[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;
}


If not then they are uninitialized and point anywhere.

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

[homerun4711]

Dammit, I am so stupid! :)
The problem was the same that I had a few days ago.

I declared a private variable in the header file


DataManager* dmgr;

But instead of initializing this one I created and initialized a new one in the constructor:


DataManager* dmgr = new DataManager();

so the other one was still Null.

Solution is


dmgr = new DataManager();

in constructor.

Sorry for bothering you again with that problem, I should have known better :)

[pkohut]

Hm, just tried a test enabling -Wshadow -Wall in gcc and it didn't pickup the shadow variable I created.

FWIW, creating a Qt project in VC++ from the Qt Wizard sets the warning level to 1 (least chatty). I like warning level 4, but it's to noisy with Qt, so have to set it to 3.

[nroberts]

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


Wysota showed you where to learn about them. Please don't overuse them. They do have a lot of the same issues as globals in that you create a dependency on a particular instance when you use them. Believe it or not, you are usually better off doing all the extra, apparently superfluous typing necessary to pass the variable around as a parameter. Too often you end up being wrong about only needing the one thing.

For example, a product I work on regular used a global variable for the document pointer. All well and good when you're a product that can only work on a single document. We wanted to extend to MDI for the longest time and where unable to because use of this global was EVERYWHERE. Using a singleton pattern will not save you from this problem (there's a particular kind that could but it's not a newbie level thing) and others like it.

Reserve singletons to things like AbstractFactory objects (which you're probably also unfamiliar with) or other objects that are *conceptually required* to be unique in a program, like QApplication. Don't use singletons (nor globals either) for things that you are saying to yourself, "Gee, I only *need* one."


Hm, just tried a test enabling -Wshadow -Wall in gcc and it didn't pickup the shadow variable I created.

FWIW, creating a Qt project in VC++ from the Qt Wizard sets the warning level to 1 (least chatty). I like warning level 4, but it's to noisy with Qt, so have to set it to 3.

There's actually only a couple warnings that need to be turned off and they're meaningless anyway. Pay attention to the number of the warning, go to Advanced in the C++ configuration, and add those numbers to the ignore warning flags. If I'm wrong about its location (no VC in front of me) then just search for it in there....it's there.

It's a very good idea to enforce the "no warnings at highest warning level" project standard but you do end up having to turn some of the more stupid errors off.