Review Request 112253: KStandardItemListWidgetInformant/KFileItemListWidgetInformant: do not call the model's data(int) method if only the "text" is needed

[Emmanuel Pescosta]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112253/#review38760
-----------------------------------------------------------



dolphin/src/kitemviews/kfileitemlistwidget.cpp
<http://git.reviewboard.kde.org/r/112253/#comment28612>

    Why two casts?
    
    Wouldn't it be better if you do the type cast first and check it afterwards?
    
    KFileItemModel* fileItemModel = qobject_cast<KFileItemModel*>(view->model());
    Q_ASSERT(fileItemModel);



dolphin/src/kitemviews/kstandarditemlistwidget.cpp
<http://git.reviewboard.kde.org/r/112253/#comment28613>

    Const Ref



dolphin/src/kitemviews/kstandarditemlistwidget.cpp
<http://git.reviewboard.kde.org/r/112253/#comment28614>

    .value("text") instead of ["text"]


- Emmanuel Pescosta


On Aug. 24, 2013, 6:54 p.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112253/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2013, 6:54 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Description
> -------
> 
> As I said on the mailing list some time ago, I think we can reduce the memory usage considerably by lazy-loading most of the data for each item. However, this only makes sense if we reduce the number of calls to the model's data() method, which constructs the full QHash containing all data, as much as possible.
> 
> Currently, KStandardItemListWidgetInformant calls the data() method for every single item, but the full data is actually only needed for the size calculation in Compact View. In Details View, no data are needed at all to determine the size required for the item, and in Icons View, only the name is needed.
> 
> This patch makes it possible for subclasses of KStandardItemListWidgetInformant to provide an alternative way to obtain the "text", to prevent calling the data() method of the model.
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kfileitemlistwidget.h 24c6778 
>   dolphin/src/kitemviews/kfileitemlistwidget.cpp 3a77241 
>   dolphin/src/kitemviews/kstandarditemlistwidget.h 4bf6116 
>   dolphin/src/kitemviews/kstandarditemlistwidget.cpp 2a89004 
> 
> Diff: http://git.reviewboard.kde.org/r/112253/diff/
> 
> 
> Testing
> -------
> 
> Works. No regressions found so far.
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20130827/e22f183b/attachment.htm>