[Qt-interest] QAbstractProxyModel methods index, parent, rowCount, columnCount not implemented?

[Andre Somers]

Hi Stephen,

Thank you very much for your comments. They are appreciated.

Op 3-6-2011 21:01, Stephen Kelly schreef:
>
> I've read the code now.
>
>
> I noticed in
> getCombinedChildrenCheckState(QModelIndex sourceIndex) const
>
> you use sourceIndex.child(0,0).
>
> This won't return what you expect if sourceIndex is invalid. The solution is
>
> if (sourceIndex.isValid())
>    sourceIndex.child(0,0);
> else
>    model->index(0, 0, sourceIndex);
>
> But of course if you have to do that anyway, the better solution is to just
> use model->index(0, 0, sourceIndex); unconditionally. I think child() should
> be removed in Qt5.
Perhaps you are right that using model->index(0,0,sourceIndex); would be 
better, but there is no error there. Just above, I check if the number 
of rows is bigger than 0 (and that would be an error already). At least, 
I /think/ that calling rows() on a model with an invalid QModelIndex 
returns 0, right? Hmmm... on second thought, perhaps that is true on a 
well implemented model, but how many custom tree models are well 
implemented? Better add an explicit check. Fixed.

>
> In checkedState() you seem to add indexes in the source model to the data
> members of the CheckableProxyModelState, but then you use mapToSource on the
> same indexes.
Oops, this is embarrassing. You are absolutely right. I will fix that.
> I'm still surprised there's no better way using things like QItemSelection
> etc to achieve what you're trying to do, though I've never attempted to
> implement the 'partially checked nodes when 1 or more children are checked'
> or related features.
I used a QItemSelectionModel on an earlier attempt. That one was limited 
to lists and tables for exactly this reason: you can only store a 
boolean (selected or not selected) in such a model. To work efficiently, 
I needed something more flexible. As you can see, I am storing two 
four-valued states for each node in the store now (not for each checked 
node per se though).
> Nothing else jumped out to me as something that will break in an obvious
> situation, but I don't really like the API of creating and passing in
> QModelIndexLists.
I am open to suggestions for an alternative. This is the only way I 
could come up with that allows getting multiple lists of indices 
efficiently (short of methods where you are forced to pass in multiple 
lists at once). Changing the API to just /return/ such a list would 
result in the situation where the state can change in between the calls 
(as was the case in the previous version). The user is free to abuse the 
returned QModelIndexLists, of course, but there is little I can do to 
stop that. Note that even QItemSelectionModel has API that returns such 
lists, so that class is just as vunerable if not more so than this one.
> There's also (unlikely I guess) ways to break it like this maybe:
>
>      CheckableProxyModelState&state = m_checkProxy->checkedState();
That doesn't compile. The versions to circumvent the immediate use of 
CheckableProxyModelState I could come up with all result in compilation 
errors, but perhaps I am not creative enough and someone will come up 
with one what does compile anyway.

> Finally, it looks like you could use a string template system in
> MainWindow::selectedItemsChanged :)
:-)
I guess I could. Suggestions? ;-)
I actually thought about it, but decided that would be too heavy for 
simple example like this.

Thanks again for your input!

André