model and itemselectionmodel, wrong item count

[Talei]

Hello,
I have problem with selection inside model/views (ListView and TableView). The problem is that I want to remove from QStandardItemModel selected rows, everything works fine, except that some selected items are not removed. I spend few days looking at the code/reading but couldn't find solution.

So I do this:
assign model and itemselectionmodel to views (for clarity sake i post only TableView code) like this:

tableView->setModel( &model );      //setup model before show()
    tableView->setSelectionBehavior( QAbstractItemView::SelectRows );
        smodelTable = new QItemSelectionModel( &model );
        tableView->setSelectionModel( smodelTable ); //I use atm. two separate selectionmodels only to figure out the problem

tableView->setModel( &model );      //setup model before show()
    tableView->setSelectionBehavior( QAbstractItemView::SelectRows );
        smodelTable = new QItemSelectionModel( &model );
        tableView->setSelectionModel( smodelTable ); //I use atm. two separate selectionmodels only to figure out the problem

To copy to clipboard, switch view to plain text mode 

then, after item are selected, I use this code to remove selected rows from model:

void MainWindow::clearSelectedItemsSlot()
{
    QModelIndexList selIndeModel;
 
    if( listView->isVisible()){
        selIndeModel = smodelList->selectedIndexes(); //for listView
    }else{
        selIndeModel = smodelTable->selectedRows(); //for tableView
    }
 
    for( int i = 0; i < selIndeModel.size(); ++i )
    {
        model.removeRow( selIndeModel[i].row() );
        fileNames.removeAt( selIndeModel[i].row() );
    }
 
    selIndeModel.clear(); //for test purpose 
    smodelList->clearSelection();
    smodelTable->clearSelection();
}

void MainWindow::clearSelectedItemsSlot()
{
    QModelIndexList selIndeModel;

    if( listView->isVisible()){
        selIndeModel = smodelList->selectedIndexes(); //for listView
    }else{
        selIndeModel = smodelTable->selectedRows(); //for tableView
    }

    for( int i = 0; i < selIndeModel.size(); ++i )
    {
        model.removeRow( selIndeModel[i].row() );
        fileNames.removeAt( selIndeModel[i].row() );
    }

    selIndeModel.clear(); //for test purpose 
    smodelList->clearSelection();
    smodelTable->clearSelection();
}

To copy to clipboard, switch view to plain text mode 

Items are indeed removed, but sometimes one stays in model, sometimes one in middle, depending what was selected.
I can't think of any reason why this is happening.

Any suggestion are more then welcome.
PS> This behaviour is same for both listView and tableView, so I suspect that my remove code is wrong

[ChrisW67]

The indexes in your selection model may be invalidated (or change what they point to) by calling removeRow() i.e. the row numbers change on rows later than than the one you remove. You want to remove the rows in descending row number order.

[Talei]

It gave me same behaviour,

for( int i = selIndeModel.size()-1; i >= 0 ; --i )
    {
        model.removeRow( selIndeModel[i].row() );
        fileNames.removeAt( selIndeModel[i].row() );
    }

for( int i = selIndeModel.size()-1; i >= 0 ; --i )
    {
        model.removeRow( selIndeModel[i].row() );
        fileNames.removeAt( selIndeModel[i].row() );
    }

To copy to clipboard, switch view to plain text mode 

when deleting only last element it works well, but when selecting i.e. odds it leaves element or skip and remove next one.

[ChrisW67]

The indexes on the QModelIndexList are not necessarily in row order, so reversing the order you extract them from the list is not the same as "descending row number order". Drop a:

qDebug() <<  "removeRow()"<<iselIndeModel[i].row() << iselIndeModel[i].data().toString() ;

qDebug() <<  "removeRow()"<<iselIndeModel[i].row() << iselIndeModel[i].data().toString() ;

To copy to clipboard, switch view to plain text mode 

inside the loop to see what rows are being deleted.

Build a QList<int> of row numbers, sort it, then run the loop.

Alternatively, this may be a case where QPersistentModelIndex is useful. Convert your QModelIndexList to a QList<QPersistentModelIndex> and use that in the loop. Indexes should remain valid.

[Talei]

Thank You very much, I used PersistantModel and everything works fine, although I need to rethink about QStringList indexes removal.

QModelIndexList selIndeModel;
    QList<QPersistentModelIndex> selPersistIndeModel;
 
    if( listView->isVisible()){
        selIndeModel.append( smodelList->selectedIndexes() );
    }else{
        selIndeModel = smodelTable->selectedRows();
    }
 
    //populate persistant indexes
    for( int i = 0; i < selIndeModel.count(); ++i)
        selPersistIndeModel.append( selIndeModel.at(i) );
 
    for( int i = selPersistIndeModel.size()-1; i >= 0; --i )
    {
        //qDebug() <<  "removeRow()"<< selIndeModel[i].row();
        model.removeRow( selPersistIndeModel[i].row() );
        fileNames.removeAt( selPersistIndeModel[i].row() );
    }

QModelIndexList selIndeModel;
    QList<QPersistentModelIndex> selPersistIndeModel;

    if( listView->isVisible()){
        selIndeModel.append( smodelList->selectedIndexes() );
    }else{
        selIndeModel = smodelTable->selectedRows();
    }

    //populate persistant indexes
    for( int i = 0; i < selIndeModel.count(); ++i)
        selPersistIndeModel.append( selIndeModel.at(i) );

    for( int i = selPersistIndeModel.size()-1; i >= 0; --i )
    {
        //qDebug() <<  "removeRow()"<< selIndeModel[i].row();
        model.removeRow( selPersistIndeModel[i].row() );
        fileNames.removeAt( selPersistIndeModel[i].row() );
    }

To copy to clipboard, switch view to plain text mode 

And question, did I do it correctly or maybe there are some better way of ding it?

[ChrisW67]

It works... that's the main thing.

You may be able to create selPersistIndeModel in one hit at line 5/7:

selPersistIndeModel.append( smodelList->selectedIndexes() );

selPersistIndeModel.append( smodelList->selectedIndexes() );

To copy to clipboard, switch view to plain text mode 

but I'm not sure if the compiler will complain or do an implicit conversion for you.

There are probably more elegant methods, but someone else will have to point them out.

[Talei]

Yes I agree, but there is also fine line between working efficiently and not. I ask only because I try to learn "good programming practice".
Also I had problem with QStringList so I implemented You previous suggestion, (QList<int>)

Here is a code that deletes selected items from QStringList and removes rows from model

void MainWindow::clearSelectedItemsSlot()
{
    QModelIndexList selIndeModel;
 
    //load selection indexes from table/list views
    if( listView->isVisible()){
        selIndeModel = smodelList->selectedIndexes();
    }else{
        selIndeModel = smodelTable->selectedRows();
    }
 
    QList<int> lint;
    //populate lint
        for( int i = 0; i < selIndeModel.count(); ++i)
            lint.append( selIndeModel[i].row() );
 
    //sort lint list and remove string at that pos from back
    //that way indexes don't shift
        qSort( lint.begin(), lint.end() );
 
        for( int i = lint.size()-1; i >= 0; --i )
        {
            model.removeRow( lint[i] );
            fileNames.removeAt( lint[i] );
        }
 
    smodelList->clearSelection();
    smodelTable->clearSelection();
}

void MainWindow::clearSelectedItemsSlot()
{
    QModelIndexList selIndeModel;

    //load selection indexes from table/list views
    if( listView->isVisible()){
        selIndeModel = smodelList->selectedIndexes();
    }else{
        selIndeModel = smodelTable->selectedRows();
    }

    QList<int> lint;
    //populate lint
        for( int i = 0; i < selIndeModel.count(); ++i)
            lint.append( selIndeModel[i].row() );

    //sort lint list and remove string at that pos from back
    //that way indexes don't shift
        qSort( lint.begin(), lint.end() );

        for( int i = lint.size()-1; i >= 0; --i )
        {
            model.removeRow( lint[i] );
            fileNames.removeAt( lint[i] );
        }

    smodelList->clearSelection();
    smodelTable->clearSelection();
}

To copy to clipboard, switch view to plain text mode 

Thank you again for suggestion, I really appreciate it.
Best regards