a fresh look at C++ models and QML

JoeCFD

        ListItem item = m_list->at(index.row());  <=== this is a copy. nothing has been changed in your model.
        switch (role) {
        case NbrRole:
            item.nbr = value.toInt();
            break;
        case DescriptionRole:
            item.description = value.toString();
            break;
        }

mzimmers

@JoeCFD I see your comment. So, I've added the following line in order to modify my model:

ListItem item = m_list->at(index.row());
switch (role) {
case NbrRole:
    item.nbr = value.toInt();
    break;
case DescriptionRole:
    item.description = value.toString();
    break;
}

if (data(index, role) != value) {
    int row = index.row();
    m_list->replace(row, item); // <== changes the model, right?
    emit dataChanged(index, index, {role});
    rc = true;
}

This appears to work. My concern is, if my updating routine changes several elements in the struct, I end up calling setData() several times, which could be inefficient in a larger example. Hence my question #4: is there a way to "group" the changes before I replace the item in my list? It appears that setItemData() would accomplish this, but I'm not sure how to use it (the doc is fairly terse).

GrecKo

You generally don't call setData inside your own logic code.
Just change the underlying data and emit dataChanged for the relevant index and roles.

setData is there when you want to let your views update your model. For example if you wanted to have a TextField in your delegate to modify the description of one of your element.

mzimmers

@GrecKo I see. So then, my updating routine would look like this:

QList<int> changedRoles;
ListItem item;

for (int i = 0; i < m_list->size(); ++i) {
    QModelIndex qmi = index(i, 0, QModelIndex());

    // initialize my temporary item from the model.
    item.nbr = m_list->at(i).nbr;
    item.description = m_list->at(i).description;

    // update the temporary item.
    item.nbr++;
    changedRoles.append(NbrRole);

    QChar qc = QChar(item.nbr + 0x40);
    item.description.append(qc);
    changedRoles.append(DescriptionRole);

    // replace the list item.
    m_list->replace(i, item);
    emit dataChanged(qmi, qmi, changedRoles);
}

And, to return to my question #4, I guess this solves the problem of multiple replaces and signals.

So, does this look about right? Any room for improvement?

Thanks...

GrecKo

@mzimmers That's not how I would write it.
Your for loop is quite synthetic but here's how I would do the same

for (ListItem& item : m_list) { // use range for loop, also get a reference to your items in your list
    item.nbr++; // edit the item reference in place
    QChar qc = QChar(item.nbr + 0x40); // ? not sure what you wanted to do there but I copied it
    item.description.append(qc);
}
// if you modify contiguous items at the same time, emit dataChanged once for all the range.
emit dataChanged(index(0), index(m_list.size() - 1), {NbrRole, DescriptionRole});

mzimmers

@GrecKo I like some things about your approach better than mine, but it leaves a small issue: without a conventional loop, how do I know what row I'm working on?

In my real app, the updates are conditional; it would look a little more like this:

QList<int> changedRoles;

for (ListItem &item: *m_list) {
        QModelIndex qmi = index(i, 0, QModelIndex()); // how to get row?

    if (something) {
        item.nbr++;
        changedRoles.append(NbrRole);
    }
    if (something else) {
        QChar qc = QChar(item.nbr + 0x40;
        item.description.append(qc);
        changedRoles.append(DescriptionRole);
    }
    if (changedRoles.size() > 0) {
        emit dataChanged(qmi, qmi, changedRoles);
    }
}

Is there some magic way to get the row number from within your loop, so I can create a QModelIndex for the signal?

Thanks...

EDIT:

Evidently, C++20 would give me the ability to do what I was asking for, but since I'm using C++17, I guess I can just do it manually:

void ListModel::update()
{
    QList<int> changedRoles;
    int row;

    row = 0;

    for (ListItem &item: *m_list) {
        QModelIndex qmi = index(row, 0, QModelIndex());

        // update the temporary item.
        item.nbr++;
        changedRoles.append(NbrRole);

        QChar qc = QChar(item.nbr + 0x40);
        item.description.append(qc);
        changedRoles.append(DescriptionRole);

        if (changedRoles.size() > 0) {
            emit dataChanged(qmi, qmi, changedRoles);
        }
        row++;
    }
}

@GrecKo I saw your comment about minimizing signals if contiguous items are changed, but in my real app, that's rarely going to be the case, and I think trying to reduce the number of signals would complicate the logic, so I'll probably leave that part as it is. Apart from this, how does this look to you?

mzimmers

Time for question #5:

I think I have my model working correctly now. I use it in a couple of QML screens. One works fine (I can get the fields I need):

ListView {
    model: spaceModel
    delegate: SpaceCard {
        titleText: name // one of my role names

and the name shows up in the SpaceCard just fine.

When I try to use it like this, though:

TabBar {
    Repeater {
        model: spaceModel
        delegate: TabButton {
            contentItem: Text {
                text: name // one of my role names

The name field remains blank. Could this be because I'm using a repeater to populate my TabBar -- from the docs:

The Repeater type creates all of its delegate items when the repeater is first created.

This idea doesn't really hold water, because my tab bar is indeed updating (verified by prepending a string to my "name" field, but...I can't think of anything else. Any ideas why this might not be properly populating the name field?

Thanks...

JoeCFD

@mzimmers I guess this will work as well.

void ListModel::update()
{
    QList<int> changedRoles;
    int row{ 0 };
    for (ListItem &item: *m_list) {
        QModelIndex qmi = index(row++, 0, QModelIndex());

        // update the temporary item.
        item.nbr++;
        changedRoles.append(NbrRole);

        QChar qc = QChar(item.nbr + 0x40);
        item.description.append(qc);
        changedRoles.append(DescriptionRole);

        if (changedRoles.size() > 0) {
            emit dataChanged(qmi, qmi, changedRoles);
        }
    }
}

mzimmers

I've played with this, and I'm nearly sure that the problem mentioned in question #5 has to do with the use of the Repeater.

This code:

ColumnLayout {
    id: infoScreen
    ListView {
        Layout.fillHeight: true
        Layout.fillWidth: true
        model: spaceModel
        delegate: Rectangle {
            height: 100
            width: 100
            color: 'lightblue'
            border.width: 1
            Text { text: "(((" + name + ")))"}
        }
    }

Produces this (correct) screen:

But this code (note the parens):

ColumnLayout {
    TabBar {
        Repeater {
            model: spaceModel
            delegate: TabButton {
                contentItem: Text {
                    text: "(((" + name + ")))"

Produces this:

The empty parens indicate that the new entries into the model are detected in the screen, so...why isn't the name showing up?

Thanks for any ideas...

mzimmers

I came up with a workaround: I use a ListView instead of a repeater:

ListView {
    id: spaceRow
    orientation: ListView.Horizontal
    model: spaceModel
    delegate: TabButton {
        id: spaceButton
        contentItem: Text {
            text: name // from model
            MouseArea {
                anchors.fill: parent
                onClicked: {
                    spaceRow.currentIndex = index
                    console.log("spaceRow.currentIndex is " + spaceRow.currentIndex)
                }
            }
        }
    }
}

Seems to work OK. I think this is a reasonable answer to question #5, though I'd still like to know whether there's a way to accomplish this with a repeater. I also think this topic is long enough, so I'm going to close it out. Thanks to everyone who helped...