Moving items in QAbstractItemModel

[arturo182]

The only change I made is to appendChild function, it now looks like this:

void RosterItem::appendChild(RosterItem *item)
{
  item->m_parent = this;
  m_children.append(item);
}

void RosterItem::appendChild(RosterItem *item)
{
  item->m_parent = this;
  m_children.append(item);
}

To copy to clipboard, switch view to plain text mode 

All the previous code is unchanged. I tried adding beginMoveRows but it caused a crash, I tried to use it like so:

beginMoveRows(oldGroup->index(), item->row(), item->row(), newGroup->index(), newGroup->rowCount());
oldGroup->removeChild(item);
newGroup->appendChild(item);
endMoveRows();

beginMoveRows(oldGroup->index(), item->row(), item->row(), newGroup->index(), newGroup->rowCount());
oldGroup->removeChild(item);
newGroup->appendChild(item);
endMoveRows();

To copy to clipboard, switch view to plain text mode 

Maybe I should inherit QStandardItem or it's used for something else?

[wysota]

What does beginMoveRows() return?

[arturo182]

I tried moving same item three times, the first time it returned true, the second time it returned true but a debug message was displayed stating "endMoveRows: Invalid index" and the third time it returned false and crashed.

I'm sure you're right, I'm not reindexing rows' numbers correctly but I don't know how to fix it.

[wysota]

I don't like the idea of diving beneath the proxy level and extracting the internal pointer outside the model. In my opinion you should ignore the fact that the underlying model deals with some groups and contacts and you should focus solely on the top level of your model stack. Otherwise you should somehow resort the index list you get so that you can then traverse the list in a proper order that won't break your indexes. Remember that each change in the model invalidates all model indexes. You should first map all the indexes to items and then when you have them all, start moving them. It'd be best if you embedded all the logic directly in the model.

[arturo182]

I just tried this:

QList<RosterContact*> contacts;
  foreach(QModelIndex in, list) {
    QModelIndex index = proxy->mapToSource(in);
 
    if(index.isValid()) {
      if(index.data(RosterItem::TypeRole) == RosterItem::Contact) {
        RosterContact *cnt = static_cast<RosterContact*>(index.internalPointer());
        if(cnt) {
          contacts.append(cnt);
        }
      }
    }
  }
 
  foreach(RosterContact *cnt, contacts) {
    itemModel->moveToGroup(cnt, group);
  }

QList<RosterContact*> contacts;
  foreach(QModelIndex in, list) {
    QModelIndex index = proxy->mapToSource(in);

    if(index.isValid()) {
      if(index.data(RosterItem::TypeRole) == RosterItem::Contact) {
        RosterContact *cnt = static_cast<RosterContact*>(index.internalPointer());
        if(cnt) {
          contacts.append(cnt);
        }
      }
    }
  }

  foreach(RosterContact *cnt, contacts) {
    itemModel->moveToGroup(cnt, group);
  }

To copy to clipboard, switch view to plain text mode 

And it works great, no crashes and all items are moved. I know looping through contacts twice is not very efficient but I guess I don't have much choice.
What do you think of this solution?

[wysota]

It's certainly better than getting crashes. But you should really change wrap it all into your model API as using internalPointer() outside the model is just looking for trouble.