An Useful Model Proposal : KStringDataListModel

Simon Hausmann hausmann at kde.org
Fri Mar 2 12:49:36 GMT 2007


On Friday 02 March 2007 13:07:32 David Faure wrote:
> On Friday 02 March 2007, Bruno Virlet wrote:
> > Hello !
> >
> > On Thursday 01 March 2007, David Faure wrote:
> > > > At the risk of asking the obvious: Why not simply use a
> > > > QStandardItemModel?
> > > > [...]
> > >
> > > Hmpf. I admit that I didn't think of doing it that way.
> >
> > I ported my model to use QStandardItemModel. It is actually nicer.
> >
> > Please find the patch attached.
>
> Hmm I think Simon's point was that we don't really need a
> KStringDataListModel class when the applications can just use
> QStandardItemModel directly. But I lack experience with either one to see
> if the application code would look pretty much fine in both cases, or very
> ugly with the latter and very nice with the former, which would be a reason
> to have a custom class indeed.
> However I see that your model also provides moveUp/moveDown, which is
> probably not so obvious to write when using QStandardItemModel directly in
> the app code. Simon, do you agree that this makes a good reason for having
> the class in kdelibs? There are many widgets/dialogs that have
> moveup/movedown functionality, like kedittoolbar etc.

I'm not sure. Let's see:

+void KStringDataListModel::moveUp( const QModelIndex &index )
+{
+    if (!index.isValid() || index.row() == 0)
+        return;
+
+    emit layoutAboutToBeChanged();
+    QList<QStandardItem*> items = takeRow(index.row() - 1);
+    QStandardItemModel::insertRow( index.row(),  items );
+    emit layoutChanged();
+}

The signal emissions are not necessary, so it comes down the same amount of 
lines in the application:

	stringDataListModel->moveUp(index);

versus

	model->insertRow(row - 1, model->takeRow(row));

On top of that I admit that I'm not convinced about the API. The insertRow 
overload takes the index last (which is inconsistent) and it also hides all 
the nice insertRow() overloads of QStandardItemModel. Also "internal" is a 
very unfortunate method name in the public API I would say ;-)

Then there are no unit tests and there's no d-pointer. I'm sorry, I'm not 
convinced this class adds real value :}

Simon
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20070302/fb3de20b/attachment.sig>


More information about the kde-core-devel mailing list