Review Request 114847: Remove call of KFileItemModel::index() in KFileItemModelRolesUpdater::applyResolvedRoles()

[Commit Hook]

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


This review has been submitted with commit 2553e9a2f413ff546b1cdb893679ddd3f27403dc by Frank Reininghaus to branch master.

- Commit Hook


On Jan. 4, 2014, 10:39 a.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/114847/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2014, 10:39 a.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> KFileItemModel allows to find out the index of a KFileItem with its index(const KFileItem&) method. Calling this method is not extremely expensive, but it's also not free (it looks up the URL of the KFileItem in a QHash, i.e., it has to call qHash(QString) for the full URL), so it should not be called if it's not really necessary IMHO.
> 
> In KFileItemModelRolesUpdater, the member applyResolvedRoles(const KFileItem& item, ResolveHint hint) calls KFileItemModel::index(const KFileItem&), even though every caller of applyResolvedRoles() knows the index already. I propose to modify applyResolvedRoles() and let it take the index as a parameter directly.
> 
> (I will investigate ways to prevent calls of KFileItemModel::index(const KFileItem&) also in other places. My longer-term idea is that we could consider not to initialize the model's m_items hash at all unless it is really needed. There are a few situations where we cannot really circumvent this, like when the KDirLister reports that a KFileItem is refreshed or deleted, but most of the time, when entering a new folder, we could avoid the initialization of the hash, which would save both memory and time.)
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kfileitemmodelrolesupdater.h fced44a 
>   dolphin/src/kitemviews/kfileitemmodelrolesupdater.cpp d6445c9 
> 
> Diff: https://git.reviewboard.kde.org/r/114847/diff/
> 
> 
> Testing
> -------
> 
> Loading icons and previews still works correctly for me.
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

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