[Development] QAbstractItemModel::setItemData behaviour

[Christian Kandeler]

On Mon, 3 Sep 2018 09:03:06 +0000
Luca Beldi <Luca.Beldi at twentyfouram.com> wrote:

> Gentle ping as I got no answers before.
> ----------------------------------------------------------------------------
> 
> Hi everyone,
> While trying to submit a patch to fix QStringListModel::setItemData https://codereview.qt-project.org/235730 we opened a much larger discussion on QAbstractItemModel, that expanded on the forum https://forum.qt.io/topic/93207/qabstractitemmodel-setitemdata-partial-success  but that I ultimately would like your opinion on:
> 
> At the moment, QAbstractItemModel::setItemData calls setData for each role until the first failure and returns false if that happens.
> This opens up 2 issues:
> 
> *         QAbstractItemModel::setItemData returns false even when some but not all data was actually set without rolling back the changes
> 
> *         QAbstractItemModel::setItemData depends on the ordering of the roles
> 
> For the second point, an example that illustrates the problem is (QStringListModel::setItemData just uses the base class implementation):
> QStringListModel::setItemData({{Qt::DisplayRole,QVariant("test")},{ Qt::DecorationRole,QVariant(QIcon())}) will return false but actually set the string to "test"
> QStringListModel::setItemData({{Qt::EditRole,QVariant("test")},{ Qt::DecorationRole,QVariant(QIcon())}) will return false and do nothing on the model
> 
> My proposal is:
> 
> *         Remove the ordering-dependency  QAbstractItemModel::setItemData (i.e. sets all the roles it can and returns false if any of them failed)
> 
> *         Make setItemData in subclasses of QAbstractItemModel behave transaction-like (i.e. try to set all the roles, if any fails rollback the other changes and return false)
> 
> This however is technically a change of behaviour so I would like people way smarter than me to agree on the way forward.

To me, both suggestions sound better than what is done currently. And technically, the potential behavior change does not break any contracts, since the details are not specified in the documentation.


Christian