Discussing CheckableSortFilterProxyModel and CheckableProxyModel

andre

I just studied the "CheckableSortFilterProxyModel snippet":https://developer.qt.nokia.com/wiki/QSortFilterProxyModel_subclass_for_readonly_columns_columns_with_checkboxes_and_password_columns#ca0dbad92a874b2f69b549293387925e , and compared it to my own "CheckableProxyModel":https://developer.qt.nokia.com/wiki/QSortFilterProxyModel_subclass_to_add_a_checkbox.

I have some comments on the API of this otherwise very useful class, which I think can be improved.

The main issue is the way the columns to use for each of the tasks are set. I find the way to have to pass in three lists of integers rather unnatural and un Qt-ish. Why do I need to set all three of them in one go? And why pass in a list, even if I want to use only a single column?

I think it would make much more sense to introduce separate methods to add a column for each of the tasks:
@
void addCheckableColumn(int column);
void addReadOnlyColumn(int column);
void addPasswordColumn(int column);
@

You might consider adding these methods to make the columns modifiable after first setting them:
@
bool removeCheckableColumn(int column);
bool removeReadOnlyColumn(int column);
bool removePasswordColumn(int column);
void clearCheckableColumns();
void clearReadOnlyColumns();
void clearPasswordColumns();
@

Finally, you could add methods to query if a column has been set as checkable, read-only or as a password column.

Something similar goes for the null/non-null columns.

This would result in this usage code (based on the usage code from the "snippet":https://developer.qt.nokia.com/wiki/QSortFilterProxyModel_subclass_for_readonly_columns_columns_with_checkboxes_and_password_columns#c64518704ce0c0d5501a45763f464276 ):
@
CheckableSortFilterProxyModel *cfpm = new CheckableSortFilterProxyModel(this);
cfpm.addCheckableColumn( usrModel->fieldIndex("isActive") );
cfpm.addCheckableColumn( usrModel->fieldIndex("isOk") );

cfpm.addReadOnlyColumn( usrModel->fieldIndex("id") );

cfpm.addPasswordColumn( usrModel->fieldIndex("passwordFld"));

cfpm.addNonNullableColumn( usrModel->fieldIndex("isActive"));

cfpm->setSourceModel( mySqlTableModel );
myTableView->setModel(cfpm);
@

I think an API like that would feel more natural.

On the implementation side of things: I don't quite get why you copy each member of each of the lists you pass in one by one. Why not simply assign the whole list in one go?

That is, instead of doing:
@

void CheckableSortFilterProxyModel::setParameters(QList<int> boolCols, QList<int> readonlyCols, QList<int> passwordCols) {
booleanSet.clear();
readonlySet.clear();
passwordSet.clear();

if (!boolCols.isEmpty())        { foreach(int column , boolCols)     { booleanSet.append(column); } }
if (!readonlyCols.isEmpty())    { foreach(int column , readonlyCols) { readonlySet.append(column); } }
if (!passwordCols.isEmpty())    { foreach(int column , passwordCols) { passwordSet.append(column); } }

}
@

You could simply do:

@
void CheckableSortFilterProxyModel::setParameters(QList<int> boolCols, QList<int> readonlyCols, QList<int> passwordCols) {
booleanSet = boolCols;
readonlySet = readonlyCols;
passwordSet = passwordCols;
}
@

But you would not need this method if you would change the API as described above.

Comments?

andre

I had some feedback on my "CheckableProxyModel":https://developer.qt.nokia.com/wiki/QSortFilterProxyModel_subclass_to_add_a_checkbox class, that pointed out an issue with it: it was keeping QModelIndexLists for the (un-) checked items. Not for the purpose of the actual state in the model, but for the API to retreive them from outside the model/view itself. The functions to access them were ment to be called immediately following each other, but of course there was no way to make sure that that was the way it would to used.

So, I modified the API to fix that.

Instead of eight functions that all returned a QModelIndexList, there is now only one checkedState() method. Technically, checkedState() returns an CheckableProxyModelState object. This object in turn has methods that take a reference to a QModelIndexList in which the results are copied, and return a reference to the class again. This way, you can chain calls to the result object. The CheckableProxyModelState can not be assigned, copied or instantiated from outside the checkedState method, thus ensuring the results of the call are current when you use them.

You can retreive the results now using a method call like this:
@
QModelIndexList selectedFiles;
QModelIndexList selectedDirectories;
QModelIndexList unselectedFiles;
QModelIndexList unselectedDirectories;

m_checkProxy->checkedState()
        .checkedLeafIndexes(selectedFiles)
        .checkedBranchIndexes(selectedDirectories)
        .uncheckedLeafIndexes(unselectedFiles)
        .uncheckedBranchIndexes(unselectedDirectories);

@

Retreiving all the required lists in one go like above is much more efficient than making separate calls like this:

@

QModelIndexList selectedFiles;
QModelIndexList selectedDirectories;
QModelIndexList unselectedFiles;
QModelIndexList unselectedDirectories;

//WRONG! This is very inefficient!
m_checkProxy->checkedState()
        .checkedLeafIndexes(selectedFiles);
m_checkProxy->checkedState()
        .checkedBranchIndexes(selectedDirectories);
m_checkProxy->checkedState()
        .uncheckedLeafIndexes(unselectedFiles);
m_checkProxy->checkedState()
        .uncheckedBranchIndexes(unselectedDirectories);

@

I hope this fixes the issue of keeping lists of QModelIndex without compromising efficiency.

boriska

Hi. In your code, I replaced the QTreeView in QListView.

@
QFileSystemModel* fsModel = new QFileSystemModel(this);

m_checkProxy = new CheckableProxyModel(this);
m_checkProxy->setSourceModel(fsModel);

ui->listView->setModel(m_checkProxy);
ui->listView->setRootIndex(fsModel->setRootPath("c:\\Program Files")); // error occurs on this line(i need list this directory)

@

when I run, an error "Microsoft Visual C++ runtume library"

Console:
@
QSortFilterProxyModel: index from wrong model passed to mapToSource
ASSERT: "!"QSortFilterProxyModel: index from wrong model passed to mapToSource"" in file itemviews/qsortfilterproxymodel.cpp, line 365
Invalid parameter passed to C runtime function.
@
Why is this bug? the description says "The model is especially suited for use on tree-type models, but will work just as well on tables or lists"

Thanks.
Invalid parameter passed to C runtime function.

andre

You try to set an index from the file system model on the list view, which is displaying the CheckableProxyModel. You need to map the index returned by fsModel->setRootPath to the corresponding index in CheckableProxyModel.

boriska

Thank you for your reply.

andre

Check out the QSortFilterProxyModel::mapFromSource() method, and use it with CheckableProxyModel instead.

boriska

Could you detail or show me exmple of using please ?

andre

What did you try already? It seems to me that the methods are quite clear. All you need to do, is realize that a QModelIndex is bound to a specific model, that a view is bound to a specific model too, and that a proxy model is still a different model. That means that you sometimes need to 'translate' qmodelindexes back and forth if you use proxy models.

mariolat

Sorry to resume this old thread, but I can't make CheckableProxyModel work with Qt 5.2.
I downloaded from git@gitorious.org:checkableproxymodel/checkableproxymodel.git the lastest version but I couldn't select or deselct the items on the treeview for the bundled example. The only changes I made was for QT5 compatibility (QT += widgets in .pro file and #include <QApplication> instead of QtGui/QApplication)

Anyone succeded on using this class with QT 5.2 ?

EDIT : I forgot to explain where the problem lies and when it occours. It happens only for directories and not files in the treeview.
The part of CheckableProxyModel which behaves differently is
@
CheckableProxyModel::TreeCheckState treeState(CheckableProxyModel::Checked);
if (state == Qt::Unchecked) {
treeState = CheckableProxyModel::Unchecked;
} else if (state == Qt::PartiallyChecked) {
qWarning() << "Unexpected new tree state.";
return false;
}
@

It constantly prints the "Unexpected new tree state." string.

By setting the

@CHECKABLEPROXYMODEL_DEBUG@

define the state is set to 2 (which corresponds on PartiallyChecked on the enum Qt::CheckState)
After some invstigations, I've seen that the differences in 5.1.1 source and 5.2.0 for QItemDelegate may be the source of this problem, in particular
For QT 5.1.1
@QItemDelegate::editorEvent(....){
...
...
Qt::CheckState state = (static_castQt::CheckState(value.toInt()) == Qt::Checked
? Qt::Unchecked : Qt::Checked);
return model->setData(index, state, Qt::CheckStateRole);
}
@

while for QT 5.2.0 is

@QItemDelegate::editorEvent(....){
...
...
Qt::CheckState state = static_castQt::CheckState(value.toInt());
if (flags & Qt::ItemIsTristate)
state = ((Qt::CheckState)((state + 1) % 3));
else
state = (state == Qt::Checked) ? Qt::Unchecked : Qt::Checked;
return model->setData(index, state, Qt::CheckStateRole);
}
@

Thank you in advance