Review Request 124331: New proxy: KExtraColumnsProxyModel, allows to add columns to an existing model.

[David Faure]

> On July 14, 2015, 4:58 p.m., Milian Wolff wrote:
> > src/kextracolumnsproxymodel.cpp, line 44
> > <https://git.reviewboard.kde.org/r/124331/diff/3/?file=385295#file385295line44>
> >
> >     these two really should be QVector. As far as I can see, you do not interact with the Qt API and thus need QList for them, or am I missing something? QModelIndex is too large, and thus you'll allocate every item in the list on the heap. int is, on 64bit, too small and thus you waste space (factor of 2).

This isn't QModelIndex, but QPersistentModelIndex, which has a d pointer, so its size is that of a pointer, no problem there.
You're right about int though. QIdentityProxyModel has the same problem then.
You're right about this being internal.

I'll change it to QVector for both, although I maintain that the QPMI one was fine ;)


> On July 14, 2015, 4:58 p.m., Milian Wolff wrote:
> > src/kextracolumnsproxymodel.cpp, line 85
> > <https://git.reviewboard.kde.org/r/124331/diff/3/?file=385295#file385295line85>
> >
> >     here and elsewhere: why not use the newstyle connect syntax?

I tried, doesn't work with private slots. &MyQObject::method requires 'method' to be in MYQObject, but with Q_PRIVATE_SLOT the method is in the private class, while the QObject is the public class.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124331/#review82500
-----------------------------------------------------------


On July 14, 2015, 10:42 a.m., David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124331/
> -----------------------------------------------------------
> 
> (Updated July 14, 2015, 10:42 a.m.)
> 
> 
> Review request for KDE Frameworks, Jesper Pedersen and Stephen Kelly.
> 
> 
> Repository: kitemmodels
> 
> 
> Description
> -------
> 
> REVIEW: 124331
> 
> 
> Diffs
> -----
> 
>   autotests/CMakeLists.txt f98e87d49b965998f31c5ede1fefb03b159a150f 
>   autotests/kextracolumnsproxymodeltest.cpp PRE-CREATION 
>   autotests/test_model_helpers.h a44db8a9cb985f69b52be96f0ffe389ebd903d6f 
>   src/CMakeLists.txt 82d776f1fcc2f7b15e74f0ed47702ed570ce9892 
>   src/kextracolumnsproxymodel.h PRE-CREATION 
>   src/kextracolumnsproxymodel.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124331/diff/
> 
> 
> Testing
> -------
> 
> Wrote autotests (as extensive as possible); used this in one real project, AFAIK Jesper (blackie) did as well.
> 
> 
> Thanks,
> 
> David Faure
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150715/eb24213c/attachment.html>