How to correctly re-assign a pointer?

[homerun4711]

Hello!

I started another topic yesterday, but I guess it went far into detail.
My questions comes down to this:

How can I correctly re-assign a pointer?

In my case, I have a QtDesigner-Layout, which creates a QTableView:



public:
QTableView *tableView;
...
void setupUi(QDialog *AddressBook)
{
tableView = new QTableView(AddressBook);
...
}


In the class which uses this layout I want to pass a pointer to a pre-defined QTableView to the one created by the layout.

AddressBook-class:

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

DataManager-class


...
QTableView *m_customerTableView;
QTableView *customerTableView();
...
m_customerTableView = new QTableView;
...
QTableView *DataManager::customerTableView()
{
return m_customerTableView;
}
...

I guess to avoid memory leaks I have to delete the original tableView created by QtDesigner.

I tried the following but could get it running, since I am not so familiar with pointers yet. (coming from Java) So there could be a misunderstanding on how pointers work.

In AddressBook-class:



delete tableView;
QTableView *tableView = dmgr->customerTableView();


I hope you can help me.

Kind regards,
HomeR

[high_flyer]

You are shadowing your member pointer.
There is no need to redeclare it:


delete tableView;
/*QTableView * */ tableView = dmgr->customerTableView();

[homerun4711]

Thanks for you anwser.
I tried what you said, but it's still not working.

Does shadowing mean, that a global variable is hidden because a local variable is used instead?

What exactly happens here?

delete tableView;
/*QTableView * */ tableView = dmgr->customerTableView();

As far as I unterstand, delete tableView does remove the QTableView object created in the QtDesigner-file ui_addressbook.h :


class Ui_AddressBook
{
public:
QTableView *tableView;
..
}

Then my prepared QTableView returned by dmgr->customerTableView(); is assigned - but assigned to what? There is no other variable named tableView existing, I guess. Do I have to define an additional one with the same name in
AddressBook.h ?

Right now my code is


delete tableView;
Q_CHECK_PTR(tableView);
Q_CHECK_PTR(dmgr->customerTableView());
tableView = dmgr->customerTableView();

Why does die application not quit on "Q_CHECK_PTR(tableView)" after "delete tableView" ?

[high_flyer]

I tried what you said, but it's still not working.
please define what 'not working' mean here.


Does shadowing mean, that a global variable is hidden because a local variable is used instead?
Yes, but instead of 'global' here its in the class scope. (so its only visible in the scope of the class).


What exactly happens here?
Well, unless there is more code that I havn't seen then:
In the very first code segment you posted in the firt post, you declare

QTableView *tableView
As a class member.
So if this code:


delete tableView;
/*QTableView * */ tableView = dmgr->customerTableView();

Is in the same clase, then tableViewe is already defined, and defining it again in the method, would shadow the one defined in the class header, and the address will be assigned to the local shadow, which gets destroyed when the method exits.
Note - the POINTER gets destroyed, but the address allocated still lives - and this called a dangling pointer - which is a memory leak - since you can't get it and delete it any more. (gets deleted when the memory of the application is returned)


Do I have to define an additional one with the same name in
AddressBook.h
No, and if you would, you would get a complication error.



As far as I unterstand, delete tableView does remove the QTableView object created in the QtDesigner-file ui_addressbook.h :
Yes and no.
'delete' deletes the object pointed to by the poiter (calling its destructor etc).
In your case, its ture, that the first allocation is done in the ui file.


Then my prepared QTableView returned by dmgr->customerTableView(); is assigned - but assigned to what? There is no other variable named tableView existing, I guess.
Don't guess - be sure! ;)


Why does die application not quit on "Q_CHECK_PTR(tableView)" after "delete tableView" ?
because delete does not set the deleted pointer to NULL.
So you should do:


delete tableView;
Q_CHECK_PTR(tableView); //will go through, since tableViewer is not NULL (but filled with garbage)
tableView = NULL;
Q_CHECK_PTR(tableView); // this will assert!
Q_CHECK_PTR(dmgr->customerTableView());
tableView = dmgr->customerTableView();

[homerun4711]

Thanks for your explanations, some things concerning pointers became clearer to me.


please define what 'not working' mean here.

My plan is to show a model listing customers.
This is defined in the QtDesigner-file, which is inherited by my AddressBook-class.

ui_addressbook.h

QTableView *tableView
...
tableView = new QTableView(AddressBook);
tableView->setObjectName(QString::fromUtf8("tableView"));


This is defined in the QtDesigner-file, which is inherited by my AddressBook-class.


Don't guess - be sure!

The AddressBook-class has no other member using the same name. I am sure :)

After deleting the object created by the QtDesigner-File using

delete tableView;
now the area beforehand used by the table in my user interface is empty. Not even an empty QTableView. Well, this is
expected as far as I unterstand this now.

Here is some more code from the DataManager-class:

DataManager.h


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


DataManager.cpp


model_customer = new QStandardItemModel;
model_customer->setObjectName("Customers");
....
m_customerTableView = new QTableView;
m_customerTableView->setModel(model_customer);

QTableView *DataManager::customerTableView()
{
return m_customerTableView;
}

...

Well, using


delete tableView;
tableView = dmgr->customerTableView();

to set the prepared QTableView from my DataManager-class does not show the QTableView in the GUI.

Sorry, I know I should ask more precise questions, but I hope you can help me.
Just tell me if you need more informations. Thanks!

Added after 13 minutes:

Ok, I rewrote the function returning the QTableWidget, it's working now.
I forgot to pass the correct parent. *stupid*stupid*

Soultion was

QTableView *DataManager::customerTableView(QWidget* parent)
{
m_customerTableView = new QTableView(parent);
m_customerTableView->setModel(model_customer);
m_customerTableView->setObjectName(QString::fromUtf8("tableView"));
m_customerTableView->setGeometry(QRect(10, 10, 701, 191));
return m_customerTableView;
}

and to call it using


tableView = dmgr->customerTableView(this);

Thanks a lot for your help and your explanations!

[high_flyer]

most welcome! :)

[nroberts]

You are shadowing your member pointer.

I understand what you're saying but I've never heard that word used to describe what you're describing. I think a more common term there might be "hiding". Could matter in google searches for anyone trying to learn more about it.



Does shadowing mean, that a global variable is hidden because a local variable is used instead?

In C++, if you declare a new name within a sub-scope of any scope that already has that name you will cause the old one to be hidden. This can include the global scope but can also include any other scope that you are currently in. With class members this has some interesting and sometimes confusing implications. You'll want to learn more about it.

[pkohut]

The AddressBook-class has no other member using the same name. I am sure :)

You should still check, dereferencing a NULL pointer is bad. What works today may not work tomorrow or next month after you've made changes to the code, and checking the returned pointer will find the problem immediately. If you know that the pointer is 100% always suppose to return a value other than NULL then use can use Q_ASSERT, otherwise use if statements.



Well, using


delete tableView;
tableView = dmgr->customerTableView();
Ok, I rewrote the function returning the QTableWidget, it's working now.
I forgot to pass the correct parent. *stupid*stupid*

Soultion was

QTableView *DataManager::customerTableView(QWidget* parent)
{
m_customerTableView = new QTableView(parent);
m_customerTableView->setModel(model_customer);
m_customerTableView->setObjectName(QString::fromUtf8("tableView"));
m_customerTableView->setGeometry(QRect(10, 10, 701, 191));
return m_customerTableView;
}
and to call it using

tableView = dmgr->customerTableView(this);

This is bad juju. What happens if you call "delete tableView" without the next call to "customerTableView"? What state is m_customerTableView variable left in? How about if there are other member functions of DataManager that work with m_customerTableView and what it points to is deleted outside of the DataManger class?

A slightly better implementation than the current.


QTableView * DataManager::DeleteCustomerTableView(void)
{
delete m_customerTableView;
m_customerTableView = NULL;
return m_customerTableview;
}
QTableView *DataManager::customerTableView(QWidget* parent)
{
DeleteCustomerTableView();
m_customerTableView = new QTableView(parent);
m_customerTableView->setModel(model_customer);
m_customerTableView->setObjectName(QString::fromUtf8("tableView"));
m_customerTableView->setGeometry(QRect(10, 10, 701, 191));
return m_customerTableView;
}
...
// delete tableView; DataManager handles the delete, this call not needed.
tableView = dmgr->customerTableView(this);

There is still a problem with the code above as it is possible to dereference a returned NULL pointer from new'd QTableView (out of memory).

Rules for staying out of trouble when programming in C/C++.
1) If a function returns an error code, check the returned value, never assume the function call was successful.
2) Always check returned pointers to ensure they are not NULL, handle accordingly.
3) If a class owns a pointer, don't delete the pointer outside of the class.
4) If a class hands ownership of pointer to another class, apply rule 3.

Qt's object ownership tree might blur #3 a bit. Still DataManager should handle the deletion of the object, so added DeleteCustomerTableView for this purpose.


tableView = dmgr->DeleteCustomerTableView();

Writing defensible code is TEDIOUS WORK (actually it's second nature if you do it all time) and adds lots of extra lines of code, however the benefits are huge. When the app starts crashing during development the error will be easier to find. When the application is deployed to outside users (with varying system architecture) defensible coding will enable you to determine the problem. Defensible coding allows errors to be handled gracefully, did a new call return NULL (out of memory) then maybe some resources can be freed and new called again. Do you check the return value from QObject::connect(...), if not how do you now the connection was successful?



The AddressBook-class has no other member using the same name. I am sure :)

You should still check, dereferencing a NULL pointer is bad. What works today may not work tomorrow or next month after you've made changes to the code, and checking the returned pointer will find the problem immediately. If you know that the pointer is 100% always suppose to return a value other than NULL then use can use Q_ASSERT, otherwise use if statements.



Well, using


delete tableView;
tableView = dmgr->customerTableView();
Ok, I rewrote the function returning the QTableWidget, it's working now.
I forgot to pass the correct parent. *stupid*stupid*

Soultion was

QTableView *DataManager::customerTableView(QWidget* parent)
{
m_customerTableView = new QTableView(parent);
m_customerTableView->setModel(model_customer);
m_customerTableView->setObjectName(QString::fromUtf8("tableView"));
m_customerTableView->setGeometry(QRect(10, 10, 701, 191));
return m_customerTableView;
}
and to call it using

tableView = dmgr->customerTableView(this);

This is bad juju. What happens if you call "delete tableView" without the next call to "customerTableView"? What state is m_customerTableView variable left in? How about if there are other member functions of DataManager that work with m_customerTableView and what it points to is deleted outside of the DataManger class?

A slightly better implementation than the current.


QTableView * DataManager::DeleteCustomerTableView(void)
{
delete m_customerTableView;
m_customerTableView = NULL;
return m_customerTableview;
}
QTableView *DataManager::customerTableView(QWidget* parent)
{
DeleteCustomerTableView();
m_customerTableView = new QTableView(parent);
m_customerTableView->setModel(model_customer);
m_customerTableView->setObjectName(QString::fromUtf8("tableView"));
m_customerTableView->setGeometry(QRect(10, 10, 701, 191));
return m_customerTableView;
}
...
// delete tableView; DataManager handles the delete, this call not needed.
tableView = dmgr->customerTableView(this);

There is still a problem with the code above as it is possible to dereference a returned NULL pointer from new'd QTableView (out of memory).

Rules for staying out of trouble when programming in C/C++.
1) If a function returns an error code, check the returned value, never assume the function call was successful.
2) Always check returned pointers to ensure they are not NULL, handle accordingly.
3) If a class owns a pointer, don't delete the pointer outside of the class.
4) If a class hands ownership of pointer to another class, apply rule 3.

Qt's object ownership tree might blur #3 a bit. Still DataManager should handle the deletion of the object, so added DeleteCustomerTableView for this purpose.


tableView = dmgr->DeleteCustomerTableView();

Writing defensible code is TEDIOUS WORK (actually it's second nature if you do it all time) and adds lots of extra lines of code, however the benefits are huge. When the app starts crashing during development the error will be easier to find. When the application is deployed to outside users (with varying system architecture) defensible coding will enable you to determine the problem. Defensible coding allows errors to be handled gracefully, did a new call return NULL (out of memory) then maybe some resources can be freed and new called again. Do you check the return value from QObject::connect(...), if not how do you now the connection was successful?



The AddressBook-class has no other member using the same name. I am sure :)

You should still check, dereferencing a NULL pointer is bad. What works today may not work tomorrow or next month after you've made changes to the code, and checking the returned pointer will find the problem immediately. If you know that the pointer is 100% always suppose to return a value other than NULL then use can use Q_ASSERT, otherwise use if statements.



Well, using


delete tableView;
tableView = dmgr->customerTableView();
Ok, I rewrote the function returning the QTableWidget, it's working now.
I forgot to pass the correct parent. *stupid*stupid*

Soultion was

QTableView *DataManager::customerTableView(QWidget* parent)
{
m_customerTableView = new QTableView(parent);
m_customerTableView->setModel(model_customer);
m_customerTableView->setObjectName(QString::fromUtf8("tableView"));
m_customerTableView->setGeometry(QRect(10, 10, 701, 191));
return m_customerTableView;
}
and to call it using

tableView = dmgr->customerTableView(this);

This is bad juju. What happens if you call "delete tableView" without the next call to "customerTableView"? What state is m_customerTableView variable left in? How about if there are other member functions of DataManager that work with m_customerTableView and what it points to is deleted outside of the DataManger class?

A slightly better implementation than the current.


QTableView * DataManager::DeleteCustomerTableView(void)
{
delete m_customerTableView;
m_customerTableView = NULL;
return m_customerTableview;
}
QTableView *DataManager::customerTableView(QWidget* parent)
{
DeleteCustomerTableView();
m_customerTableView = new QTableView(parent);
m_customerTableView->setModel(model_customer);
m_customerTableView->setObjectName(QString::fromUtf8("tableView"));
m_customerTableView->setGeometry(QRect(10, 10, 701, 191));
return m_customerTableView;
}
...
// delete tableView; DataManager handles the delete, this call not needed.
tableView = dmgr->customerTableView(this);

There is still a problem with the code above as it is possible to dereference a returned NULL pointer from new'd QTableView (out of memory).

Rules for staying out of trouble when programming in C/C++.
1) If a function returns an error code, check the returned value, never assume the function call was successful.
2) Always check returned pointers to ensure they are not NULL, handle accordingly.
3) If a class owns a pointer, don't delete the pointer outside of the class.
4) If a class hands ownership of pointer to another class, apply rule 3.

Qt's object ownership tree might blur #3 a bit. Still DataManager should handle the deletion of the object, so added DeleteCustomerTableView for this purpose.


tableView = dmgr->DeleteCustomerTableView();

Writing defensible code is TEDIOUS WORK (actually it's second nature if you do it all time) and adds lots of extra lines of code, however the benefits are huge. When the app starts crashing during development the error will be easier to find. When the application is deployed to outside users (with varying system architecture) defensible coding will enable you to determine the problem. Defensible coding allows errors to be handled gracefully, did a new call return NULL (out of memory) then maybe some resources can be freed and new called again. Do you check the return value from QObject::connect(...), if not how do you now the connection was successful?

Added after 9 minutes:


With class members this has some interesting and sometimes confusing implications.

and loops.

{
int i = 0;
.
.
.
for(int i = 0; i < someLayout->count(); ++i) {
if(someLayout->itemAt(i) == someWidget)
break;
}
someLayout->insertWidget(i, new QWidget()); // oops, inserted widget at position 0
}

[homerun4711]

Thanks for your good suggestions on writing clean code.
I have just one more question, you wrote


// delete tableView; DataManager handles the delete, this call not needed.
tableView = dmgr->customerTableView(this);

Did you mean, that DataManager automatically deletes the object created in the QtDesigner file (see code below), if tableView is assigned to another object?


public:
QTableView *tableView;
...
void setupUi(QDialog *AddressBook)
{
tableView = new QTableView(AddressBook);
...
}

Added after 25 minutes:

Hm...I added the code you provided, now the app is crashing. The debugger says that the last call was to DeleteCustomerTableView()...
what could be the problem?

[pkohut]

'Cause I'm still new to Qt and its object owner/memory system, I'm not exactly sure if the object should be deleted as described. Maybe deleteLater() should be called on the object, instead of delete. If that's the case then


QTableView * DataManager::DeleteCustomerTableView(void)
{
m_customerTableView->deleteLater();
m_customerTableView = NULL;
return m_customerTableview;
}
But, I'm really just guessing here.

ps, no idea how that post ended up triple posted.

Added after 10 minutes:


Hm...I added the code you provided, now the app is crashing. The debugger says that the last call was to DeleteCustomerTableView()...
what could be the problem?

Is m_customerTableView initialized to NULL in DataManager's constructor? If not then it is pointing to LaLa land, and the delete is blowing it up.

[homerun4711]

Is m_customerTableView initialized to NULL in DataManager's constructor?

You are right, extactly this was the problem. At first I initialized all tableViews with "new" objects but I changed that some hours ago and forgot to set the pointers to NULL.

[pkohut]

You are right, extactly this was the problem. At first I initialized all tableViews with "new" objects but I changed that some hours ago and forgot to set the pointers to NULL.

Yep, do it all the time, make some seemingly minor change and the whole thing blows up. Back to

What works today may not work (added: in a few hours) tomorrow or next month after you've made changes to the code
:o

[homerun4711]

:D you say it

[homerun4711]

Hello again!

I encountered another problem while using the DeleteCustomerTableView() you suggested

If I open, close and open the AddressBook again, the application crashes. The debugger
points to DeleteCustomerTableView(). But

If I comment the "delete m_customerTableView;" in "DeleteCustomerTableView()" everything works fine. So I think there could be a problem with a not initialized m_customerTableView. But it is set to NULL in the constructor so on the first call of AddressBook it is initialized and after that it is handled correctly, isn't it? I would be thankful if you could have another look into the code.


AddressBook::AddressBook(DataManager* dmgr, QWidget *parent): QDialog(parent)
{
//calling this for the second time crashes the app
Q_CHECK_PTR(dmgr->customerTableView());
tableView = dmgr->customerTableView(this);
}


DataManager::DataManager()
{
...
m_customerTableView = NULL; //setting to NULL in constructor
...
}


QTableView *DataManager::customerTableView(QWidget* parent)
{
DeleteCustomerTableView();

m_customerTableView = new QTableView(parent);
m_customerTableView->setModel(model_customer);
return m_customerTableView;
}


QTableView *DataManager::DeleteCustomerTableView()
{
delete m_customerTableView;
m_customerTableView = NULL;
return m_customerTableView;
}

[pkohut]

You've probably called delete on the tableView pointer in AddressBook's destructor. Instead call DeleteCustomerTableView().

Also, this is no good, because an extra delete/create operation is performed.

AddressBook::AddressBook(DataManager* dmgr, QWidget *parent): QDialog(parent)
{
//calling this for the second time crashes the app
Q_CHECK_PTR(dmgr->customerTableView());
tableView = dmgr->customerTableView(this);
}
Change to -

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

[high_flyer]

Since we are defencive programming then you should really do:


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

[homerun4711]

I haven't written a destructor. Is "delete tableView" automatically called then and thus leaving a dangling pointer which is crashing the following delete?

[pkohut]

Since we are defencive programming then you should really do:


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


Yep. It may seem redundant now because dmgr was/is presumably validated before calling into the function, but as the code base grows and more entry points to the function are added, doing the extra sanity checks will save your bacon down the road.

[high_flyer]

if tableViewe has a QObject parent (or aparent that takes ownership) then you don't have to worry about deleting it.
Otherwise, you do.

[homerun4711]

Well, I am a bit confused now, because after adding a destructor


AddressBook::~AddressBook()
{
data->DeleteCustomerTableView();
}

everything works fine and I unterstand why this is the case:
"delete tableView" is called automatically if there is no destructor and thus leaving a dangling pointer which is crashing the following delete in DeleteCustomerTableView()

But also tableView has a parent (AddressBook)


class Ui_AddressBook
{
public:
QTableView *tableView;
void setupUi(QDialog *AddressBook)
{
...
tableView = new QTableView(AddressBook);
...
}
}

and high_flyer wrote


if tableViewe has a QObject parent (or aparent that takes ownership) then you don't have to worry about deleting it.

so the crash should not happen at all?!?

[pkohut]

so the crash should not happen at all?!?

It happens because DataManager still thinks m_customerTableView is valid. Post full versions of AddressBook and DataManager (both header and cpp). It's diffecult to get the big picture at this point with just code fragments.

Added after 19 minutes:

I've got an idea, will take a few to compose my thoughts. Hold off on posting the code if you like.

[homerun4711]

One minute to late :)
Here is the complete code:

AddressBook.h

class AddressBook : public QDialog, public Ui::AddressBook
{
Q_OBJECT

public:
AddressBook(DataManager* dmgr, QWidget *parent = 0);
~AddressBook();

private:
DataManager* data;

};

#endif

AddressBook.cpp

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

Q_ASSERT(dmgr);
data = dmgr;
Q_ASSERT(data);

setupUi(this);
connect(buttonBox, SIGNAL(rejected()), this, SLOT(reject()));

tableView = dmgr->customerTableView(this);
Q_ASSERT(tableView);

}

AddressBook::~AddressBook()
{
data->DeleteCustomerTableView();
}

DataManager.h


class DataManager
{
public:
DataManager();

QStandardItemModel *customerModel();
QStandardItemModel *articlesModel();
QStandardItemModel *invoicesModel();
QStandardItemModel *invoiceEntriesModel();

QTableView *customerTableView(QWidget *parent = 0);
QTableView *articlesTableView(QWidget *parent = 0);
QTableView *invoicesTableView(QWidget* parent = 0);
QTableView *invoiceEntriesTableView(QWidget* parent = 0);

QTableView *DeleteCustomerTableView();
QTableView *DeleteArticlesTableView();
QTableView *DeleteInvoicesTableView();
QTableView *DeleteInvoiceEntriesTableView();

private:

QStandardItemModel *model_customer;
QStandardItemModel *model_articles;
QStandardItemModel *model_invoices;
QStandardItemModel *model_invoiceEntries;

QTableView *m_customerTableView;
QTableView *m_articlesTableView;
QTableView *m_invoicesTableView;
QTableView *m_invoiceEntriesTableView;

QItemSelectionModel *selectionModelCustomer;
QItemSelectionModel *selectionModelArticles;
QItemSelectionModel *selectionModelInvoices;
QItemSelectionModel *selectionModelInvoiceEntries;

QHeaderView *headerViewCustomer;
QHeaderView *headerViewArticles;
QHeaderView *headerViewInvoices;
QHeaderView *headerViewInvoiceEntries;

void setupModels();
void setupTableViews();

};

DataManager.cpp



DataManager::DataManager()
{
QTextStream out(stdout);
out << "DataManager()\n";

setupModels();
setupTableViews(); //set QTableViews to NULL

}

void DataManager::setupModels()
{

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

model_customer = new QStandardItemModel;
model_customer->setObjectName("Customers");

model_articles = new QStandardItemModel;
model_articles->setObjectName("Articles");

model_invoices = new QStandardItemModel;
model_invoices->setObjectName("Invoices");

model_invoiceEntries = new QStandardItemModel;
model_invoiceEntries->setObjectName("InvoiceEntries");

}



void DataManager::setupTableViews()
{
QTextStream out(stdout);
out << "setupTableViews()\n";

//set to NULL, else the delete later will crash the app
m_customerTableView = NULL;
m_articlesTableView = NULL;
m_invoicesTableView = NULL;
m_invoiceEntriesTableView = NULL;

}

QStandardItemModel *DataManager::customerModel()
{
Q_ASSERT(model_customer);
return model_customer;
}

QStandardItemModel *DataManager::articlesModel()
{
Q_ASSERT(model_articles);
return model_articles;
}

QStandardItemModel *DataManager::invoicesModel()
{
Q_ASSERT(model_invoices);
return model_invoices;
}

QStandardItemModel *DataManager::invoiceEntriesModel()
{
Q_ASSERT(model_invoiceEntries);
return model_invoiceEntries;
}

QTableView *DataManager::customerTableView(QWidget* parent)
{

DeleteCustomerTableView();

m_customerTableView = new QTableView(parent);
m_customerTableView->setModel(model_customer);
m_customerTableView->setGeometry(QRect(10, 10, 700, 400));
m_customerTableView->setObjectName("CustomerTableView");;
m_customerTableView->setSortingEnabled(true);
m_customerTableView->sortByColumn(0, Qt::DescendingOrder);
m_customerTableView->setSelectionMode(QAbstractItemView::SingleSelectio n);
m_customerTableView->setSelectionBehavior(QAbstractItemView::SelectRows );
selectionModelCustomer = new QItemSelectionModel(model_customer);
m_customerTableView->setSelectionModel(selectionModelCustomer);
headerViewCustomer = m_customerTableView->horizontalHeader();
return m_customerTableView;
}


QTableView *DataManager::articlesTableView(QWidget* parent)
{

DeleteArticlesTableView();

m_articlesTableView = new QTableView(parent);
m_articlesTableView->setModel(model_articles);
m_articlesTableView->setGeometry(QRect(10, 10, 700, 400));
m_articlesTableView->setObjectName("ArticlesTableView");;
m_articlesTableView->setSortingEnabled(true);
m_articlesTableView->sortByColumn(0, Qt::DescendingOrder);
m_articlesTableView->setSelectionMode(QAbstractItemView::SingleSelectio n);
m_articlesTableView->setSelectionBehavior(QAbstractItemView::SelectRows );
selectionModelArticles = new QItemSelectionModel(model_articles);
m_articlesTableView->setSelectionModel(selectionModelArticles);
headerViewArticles = m_articlesTableView->horizontalHeader();
return m_articlesTableView;
}

QTableView *DataManager::invoicesTableView(QWidget* parent)
{

DeleteInvoicesTableView();

m_invoicesTableView = new QTableView(parent);
m_invoicesTableView->setModel(model_invoices);
m_invoicesTableView->setGeometry(QRect(10, 10, 700, 400));
m_invoicesTableView->setObjectName("ArticlesTableView");;
m_invoicesTableView->setSortingEnabled(true);
m_invoicesTableView->sortByColumn(0, Qt::DescendingOrder);
m_invoicesTableView->setSelectionMode(QAbstractItemView::SingleSelectio n);
m_invoicesTableView->setSelectionBehavior(QAbstractItemView::SelectRows );
selectionModelInvoices = new QItemSelectionModel(model_invoices);
m_invoicesTableView->setSelectionModel(selectionModelInvoices);
headerViewInvoices = m_invoicesTableView->horizontalHeader();
return m_invoicesTableView;
}

QTableView *DataManager::invoiceEntriesTableView(QWidget* parent)
{
DeleteInvoiceEntriesTableView();

m_invoiceEntriesTableView = new QTableView(parent);
m_invoiceEntriesTableView->setModel(model_invoiceEntries);
m_invoiceEntriesTableView->setGeometry(QRect(10, 10, 700, 400));
m_invoiceEntriesTableView->setObjectName("ArticlesTableView");;
m_invoiceEntriesTableView->setSortingEnabled(true);
m_invoiceEntriesTableView->sortByColumn(0, Qt::DescendingOrder);
m_invoiceEntriesTableView->setSelectionMode(QAbstractItemView::SingleSelectio n);
m_invoiceEntriesTableView->setSelectionBehavior(QAbstractItemView::SelectRows );
selectionModelInvoiceEntries = new QItemSelectionModel(model_invoiceEntries);
m_invoiceEntriesTableView->setSelectionModel(selectionModelInvoiceEntries);
headerViewInvoices = m_invoiceEntriesTableView->horizontalHeader();
return m_invoiceEntriesTableView;
}



//suggestion from forum, to avoid leaks
QTableView *DataManager::DeleteCustomerTableView()
{
delete m_customerTableView;
m_customerTableView = NULL;
return m_customerTableView;
}

QTableView *DataManager::DeleteArticlesTableView()
{
delete m_articlesTableView;
m_articlesTableView = NULL;
return m_articlesTableView;
}

QTableView *DataManager::DeleteInvoicesTableView()
{
delete m_invoicesTableView;
m_invoicesTableView = NULL;
return m_invoicesTableView;
}

QTableView *DataManager::DeleteInvoiceEntriesTableView()
{
delete m_invoiceEntriesTableView;
m_invoiceEntriesTableView = NULL;
return m_invoiceEntriesTableView;
}

[pkohut]

AddressBook "has-a" QTableView, it should own it. DataManager should only reference it. So remove m_customerTableView from DataManager and move it to AddressBook.
Create a new member function in DataManager called CustomerModel.
If DataManager needs to operate on a QTableView then pass a pointer to the function.


AddressBook::AddressBook(DataManager* dmgr, QWidget *parent): QDialog(parent)
{
m_customerTableView = new QTableView(this);
m_customerTableView->setModel(dmgr->customerModel());
}

DataManager::DataManager()
{
...
...
}

QAbstractItemMode * DataManager::customerModel(void)
{
return model_customer;
}

void DataManager::someFunctionThatNeedsToWorkonQTableVi ew(QTableView * tableView)
{
tableView->doWhatEverNeedsToBeDone();
}


Of course follow proper Qt naming conventions and code defensively.

[homerun4711]

Yes, I understand what you mean, but I like to keep the QTableView central in DataManager, because it is used by more components, not only by AddressBook, to provide a uniform interface where ever in the GUI a customer list is shown.
I would prefer to fix the errors and completly understand why they happen to prevent them in the future.

[pkohut]

One minute to late :)
Here is the complete code:


Move all the table view code to the AddressBook. The views are owned by the dialog and will be automatically deleted when the dialog is dismissed. Headache gone, code simplified.


Yes, I understand what you mean, but I like to keep the QTableView central in DataManager, because it is used by more components, not only by AddressBook, to provide a uniform interface where ever in the GUI a customer list is shown.
I would prefer to fix the errors and completly understand why they happen to prevent them in the future.

Ask yourself these questions -
Does DataManager have a tableView?
Does AddressBook have a tableView?
Who should be the owner?

If DataManager does most of the work, then pass pointers of the tableView to the worker functions. In the furture, if you derive a class from QTableView, then overload the DataManager worker function with the derived classes.


class MyTableView1 : public QTableView;
class MyTableView2 : public QTableView;
AddressBook::DoSomeWork()
{
dmgr->DoSomeWork(pMyTableView1);
dmgr->DoSomeWork(pMyTableView2);
}

void DataManager::DoSomeWork(MyTableView1 * pTableView);
void DataManager::DoSomeWork(MyTableView2 * pTableView);


ps, and I see that address book is just one of many Dialog classes planned. Each of those classes will own their own table view and pass that to the data manager.

[homerun4711]

Does DataManager have a tableView?
Does AddressBook have a tableView?
Who should be the owner?


If DataManager does most of the work, then pass pointers of the tableView to the worker functions. In the furture, if you derive a class from QTableView, then overload the DataManager worker function with the derived classes.


Yes, you are right. I will do that in the future, well, maybe I will rewrite the application I am working on right now to work this way.

Thanks a lot for your help and your suggestions!

[pkohut]

Yes, you are right. I will do that in the future, well, maybe I will rewrite the application I am working on right now to work this way.

With any code, but Qt for sure, if you feel like you're fighting the code or framework you probably are. Always be open minded about refactoring a chunk of code, even if it's a 1000 lines and took a week to write. Especially when first learning a language or framework, because most of that learning is code thrashing and the what's written is probably fragile.



Thanks a lot for your help and your suggestions!
Thank you as well, this provided me with an opportunity to explore the API differently than I might have, definitely sooner.