Moving items in QAbstractItemModel

[arturo182]

I made a treeview with a model based on this example http://doc.qt.nokia.com/latest/itemv...treemodel.html And now I want to move items from one branch to another. Here's some of my code:

QModelIndexList list = selectedIndexes();
  RosterSortProxy *proxy = dynamic_cast<RosterSortProxy*>(model());
  RosterItemModel *itemModel = dynamic_cast<RosterItemModel*>(proxy->sourceModel());
 
  QString group = action->property("group").toString();
 
  foreach(QModelIndex in, list) {
    QModelIndex index = proxy->mapToSource(in);
 
    if(index.isValid()) {
        RosterContact *cnt = static_cast<RosterContact*>(index.internalPointer());
        if(cnt) {
          itemModel->moveToGroup(cnt, group);
        }
      }
  }
 
  proxy->invalidate();

QModelIndexList list = selectedIndexes();
  RosterSortProxy *proxy = dynamic_cast<RosterSortProxy*>(model());
  RosterItemModel *itemModel = dynamic_cast<RosterItemModel*>(proxy->sourceModel());

  QString group = action->property("group").toString();

  foreach(QModelIndex in, list) {
    QModelIndex index = proxy->mapToSource(in);

    if(index.isValid()) {
        RosterContact *cnt = static_cast<RosterContact*>(index.internalPointer());
        if(cnt) {
          itemModel->moveToGroup(cnt, group);
        }
      }
  }

  proxy->invalidate();

To copy to clipboard, switch view to plain text mode 

moveToGroup:

RosterItem *newGroup = groupItem(groupName);
  RosterItem *oldGroup = groupItem(item->contact()->group());
 
  if(newGroup != oldGroup) {
    oldGroup->removeChild(item);
    newGroup->appendChild(item);
 
    if((oldGroup->childCount() == 0) && (oldGroup != m_root)) {
      removeGroup(oldGroup);
    }
  }

RosterItem *newGroup = groupItem(groupName);
  RosterItem *oldGroup = groupItem(item->contact()->group());

  if(newGroup != oldGroup) {
    oldGroup->removeChild(item);
    newGroup->appendChild(item);

    if((oldGroup->childCount() == 0) && (oldGroup != m_root)) {
      removeGroup(oldGroup);
    }
  }

To copy to clipboard, switch view to plain text mode 

removeChild and appendChild just remove/append the item to group's list of children.

And the problem I'm having is sometimes when I try to move a item, the program crashes, I tried to find a pattern for those crashes but couldn't. Also sometimes not all selected items are moved. I think it's an issue with the model not being updated, but I tried layoutAboutToBeChanged() and beginMoveRows() I even tried using both beginRemoveRows() and beginInsertRows() with no luck, I don't know if I'm using them wrong or what.

Please help.

[wysota]

What is the content of removeChild() and appendChild()?

[arturo182]

Roster Item is the equivalent of TreeItem in the example, appendChild is already there I just added removeChild.

appendChild:

m_children.append(item);

m_children.append(item);

To copy to clipboard, switch view to plain text mode 

removeChild:

m_children.removeAll(item);

m_children.removeAll(item);

To copy to clipboard, switch view to plain text mode 

[wysota]

That's wrong. The model from the example was never fit to be fed live. When you manipulate the underlying data structure, you always have to call begin{Insert,Remove,Move}Rows() and end{Insert,Remove,Move}Rows().

[arturo182]

But should I do:
beginMoveRows
removeChild
appendChild
endMoveRows

or

beginRemoveRows
removeChild
endRemoveRows

beginInsertRows
appendChild
endRemoveRows

Cause I tried both and still the program is crashing. Maybe I should call those functions somewhere earlier in the code?

//edit:
Ok I think I got it, I didn't change the Item's parent, so it thought it had other parent then it really had.

One other thing, when I select multiple items in my tree view, not all of them are moved, usually one or two stay in the same group. What could be the reason for that?

[wysota]

My guess is that relative row numbers change when you start removing items and you are not taking that into account in your foreach loop.

[arturo182]

Nope, I was wrong, the problem still exists. I can only move each item once, when I try to move it the second time it gets cloned (one copy stays in old group, and second is created in new group) on third move it crashes.

I still can't find what I'm doing wrong, I'm removing the item from old group, append it to new group, change item's parent. What else do I have to do. Can some one provide me with an example please?

[wysota]

Show your current code, please.

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