[Qt-interest] Accessors for custom data on QAIM implementations (Re: Animated QProgressBar in a QTableView)

[Stephen Kelly]

Andre Somers wrote:

>   Hi,
> 
> I think you are right about your critique, but I wanted to comment on
> one issue (just for the lurkers out there ;-) )
> 
> Op 2-9-2010 19:12, Stephen Kelly schreef:
>> If you want real decoupling of model from everything else (the whole
>> point) never put additional accessors (like getEmitters) on your model.
>>
> I think the idea is that you should never put additional accessors on
> your model for the views or delegates that interact with it. That should
> all stay inside the QAbstractItemModel interface. But, for back-end code
> that interacts with the model, I think it is perfectly fine to add
> custom accessors on your model. Compare with for instance the
> QFileSystemModel. It adds quite a bit of file system specific API on the
> model class, but all of this API is for use by the back-end part of the
> code, not for use by the views.

I see the point you're making and that it is sometimes convenient to have 
type-specific data accessors, and that QFileSystemModel has accessors like 
that too for convenience, but that doesn't make it a good idea :). For a 
beginner-friendly class it's fine, but if you do complicated applications it 
doesn't make sense. It encourages people to write methods which take a 
QFileSystemModel* instead of a QAbstractItemModel* or a QModelIndex, which 
makes it hard to use the same method when you have a proxy on top. 

/**
  Returns whether the file at @p index in the QFileSystemModel @p model 
  has been modified since @p dateTime.
*/
bool SomeClass::isModifiedSince(QFileSystemModel *model, const QModelIndex 
&index, const QDateTime &dateTime)
{
  return model->lastModified(index) > dateTime;
}

Now lets say you use a proxy model which gives you a model of the children 
of a particular QModelIndex such as KSelectionProxyModel or this one:

http://lynxline.com/jongling-qt-models/

and you want to use a particular QModelIndex in that proxy with the same 
method. Here's what people do, possibly with more error checking:

bool SomeClass::isModifiedSince(QAbstractItemModel *model, const QModelIndex 
&index, const QDateTime &dateTime)
{
  QFileSystemModel *realModel = qobject_cast<QFileSystemModel*>(model);
  if (!realModel) {
    QAbstractProxyModel *proxy = qobject_cast<QAbstractProxyModel*>(model); 
    realModel = qobject_cast<QFileSystemModel*>(proxy->sourceModel());
  }

  return realModel->lastModified(index) > dateTime;
}

Someone else might use UngroupProxyModel instead of QAbstractProxyModel and 
that would be less useful. So now you can have one proxy on top of the 
QFileSystemModel and you can still use your method. What if you add another? 
Do you do the qobject_cast in a loop until you get your 'real' model? Isn't 
that throwing away any separation in the architecture?

Here's a better implementation of that method. 

/**
  ... The caller must ensure that the QModelIndex @p index is valid.
*/
bool SomeClass::isModifiedSince(const QModelIndex &index, const QDateTime 
&dateTime)
{
  Q_ASSERT(index.isValid());
  return index->data(QFileSystemModel::LastModifiedRole).toDateTime() > 
dateTime;
}

That one works no matter what proxy index is from. Remember that any proxy 
on top of a QFileSystemModel is 'A model of part of a filesystem' just like 
a QFileSystemModel is 'A model of part of a filesystem'. Views are just 
observers or consumers of the 'model of part of a filesystem' just like all 
of your API should be, so they should use the model interface which is 
QAbstractItemModel. I see no data accessors in the QFileSystemModel API 
which couldn't use data() instead. Particular methods shouldn't care whether 
it is dealing the 'root model' or a proxy on top of it. It shouldn't matter 
whether it's a view or delegate or something else. You shouldn't care what 
model a QModelIndex is from, only that it is an accessor for data. Think of 
it as a pointer, and that you implement your API to take pointers instead of 
strong types, which you then cast (which is obviosly not a good idea but 
it's a good analogy).

/**
  @p filePtr must be a QFile.
*/
bool SomeClass::isModifiedSince(QObject* filePtr, const QDateTime &dateTime)
{
  QFile *file = qobject_cast<QFile*>(filePtr);
  QFileInfo fi(*file);
  return fi.lastModified() > dateTime;
}

bool SomeClass::isModifiedSince(const QModelIndex &index, const QDateTime 
&dateTime)
{
  Q_ASSERT(index.isValid());
  QDateTime fileDateTime = qmodelIndex_cast<QDateTime>(index, 
QFileSystemModel::LastModifiedRole);
  return fileDateTime > dateTime;
}

where qmodelindex_cast is

template <typename T>
T qmodelindex_cast(index, role) {
  return index->data(role).value<T>();
}

Think of models and proxies like base classes and derived classes. If you 
have a method which reads from a file you make it take a QIODevice* instead 
of a QFile* so that you can also use it with a QLocalSocket* or a QBuffer*. 
You implement your API to use the most 'base' API possible. The more you do 
that the more you'll be able to write re-usable components which consume 
models.

> 
> Remember that a QAbstractItemModel derived class is supposed to only
> provide a view on an existing source of data in your application. It is
> an adaptor with a standardized interface between your actual data store
> and the UI views on that data. 

Not just the UI views :). Anything that accesses the model can be considered 
a view. Even a proxy is a 'view' or observer of a source model, but it also 
implements the same standard interface so that it can be 'viewed' too by 
other consumers of that standardized interface.

> In a nicely separated design, that will
> often mean that you need to define slots in your model so it can be
> signalled of changes in the data store.

Sure, but those slots can be private or Q_PRIVATE_SLOTS.

Of course, it's only API for access of custom data of particular 
QModelIndexes that's a bad idea. API for configuring the 'root' model or 
accessing its configuration is fine, like resolveSymlinks().

Sorry for the long email, but it's for the lurkers, you understand :).

All the best,

Steve.

> 
> André