CODEWITHSUNDEEP
c++c++11pointerssmart-pointers
Tipok
Tipok

Reputation: 695

smart pointers instead of pointers

Sorry for my English. I have same code:

auto windowsStack = m_windowManger->windowsStack();

auto ListModel = new QStandardItemModel();

while(!windowsStack.empty())
{
    auto window = windowsStack.top();
    auto title = QString::fromUtf8(window->title().c_str());

    auto Items = new QStandardItem(title);
    ListModel->appendRow(Items);

    windowsStack.pop();
}

ui->listView->setModel(ListModel);

everything works fine, my task is to replace the pointers to the smart pointers. I've done it more than once, my decision:

auto windowsStack = m_windowManger->windowsStack();

auto ListModel = std::shared_ptr<QStandardItemModel>();

while(!windowsStack.empty())
{
    auto window = std::shared_ptr<Window>(windowsStack.top());
    auto title = QString::fromUtf8(window->title().c_str());

    auto Items = std::shared_ptr<QStandardItem>(new QStandardItem(title));
    ListModel->appendRow(Items.get());

    windowsStack.pop();
}

ui->listView->setModel(ListModel.get());

But in the end, I get the message: The program ended unexpectedly. At the prompt, type:

ListModel->appendRow(Items.get());

New version:

auto ListModel = std::make_shared<QStandardItemModel>();
while(!windowsStack.empty())
{
    auto window = windowsStack.top();
    windowsStack.pop();
    auto title = QString::fromUtf8(window->title().c_str());
    ListModel->appendRow(new QStandardItem(title));
}

ui->listView->setModel(ListModel.get());

Upvotes: 1

Views: 205

Answers (2)

Edgar Rokjān
Edgar Rokjān

Reputation: 17483

auto ListModel = std::shared_ptr<QStandardItemModel>();

At this line you create an empty shared pointer ListModel.

Try to replace with:

auto ListModel = std::shared_ptr<QStandardItemModel>(new QStandardItemModel());

As Ryan pointed me out, It is better to use std::make_shared, which helps to reduce the amount of the code and avoid redundant memory allocations:

auto ListModel = std::make_shared<QStandardItemModel>();

NOTE:

I have just described a single mistake. Seems that there are other problems in your code. Check the Ryan's answer for more details.

Upvotes: 3

Ryan Haining
Ryan Haining

Reputation: 36882

There are a couple of problems here. The first is as pointed out in the comments and Edgar Rokyans answer, that you create ListModel as a shared_ptr to null, that is first fixed by allocating an item with make_shared

auto ListModel = std::make_shared<QStandardItemModel>();

The more sinister error is that you are creating elements within the loop, then passing non-owning pointers to ListModel which become invalid at the end of the loop when Items's destructor runs. QStandardItemModel is defined to delete the items it has so you shouldn't be using a shared_ptr to allocate items before passing them in. Instead just allocate and call appendRow. Additionally you are wrapping what you get from windowsStack in a shared_ptr, but windowsStack seems to be a copy of a stack of owning pointers, so this is a strange thing to be doing. I can't be sure, but it looks like you actually want to either use raw pointers here, or make windowsStack be a stack of shared_ptr. I can't really be sure because I don't know what windowsStack() is returning - it might be a stack of owning raw pointers to dynamically allocated memory that the caller is supposed to delete.

while(!windowsStack.empty()) {
    auto window = windowsStack.top();
    windowsStack.pop();
    auto title = QString::fromUtf8(window->title().c_str());
    ListModel->appendRow(new QStandardItem(title));
}

Upvotes: 3

Related Questions