Review Request 115018: Use the QMap iterator instead of foreach(key, map.keys()).

[Mark Gaiser]

Replying by mail since reviewboard doesn't seem to respond right now.


On Wed, Jan 15, 2014 at 2:52 PM, Frank Reininghaus <frank78ac at googlemail.com
> wrote:

>    This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115018/
>
> Ship it!
>
> Thanks for the patch! I first thought that replacing the foreach loop would probably not affect the performance much, but when iterating the keys of a QMap, rather than, e.g., the elements of a QVector, it saves the creation of the temporary list returned by itemStates.keys() (I guess that this was also the explanation that Thiago gave - maybe it was in a private discussion, Mark? I could not see any comment from Thiago here).
>
> Yes, he mailed in private.

But i don't think he minds if i paste it here since it's quite educational!

Quote from Thiago:
--------------------
It's not a matter of taste.
If you iterate using the iterators, you get O(n) complexity and O(1)
memory.
If you iterate using keys(), you get O(n log n) complexity and O(n) memory.
--------------------

It's quite amazing how educational that is. With just a few lines :)


> One could in principle move the declaration of 'it' into the for statement, but I assume that you did not do this in order to prevent that the line gets too long? Either solution is fine for me.
>
>
> - Frank Reininghaus
>
> On January 15th, 2014, 12:03 a.m. UTC, Emmanuel Pescosta wrote:
>   Review request for Dolphin.
> By Emmanuel Pescosta.
>
> *Updated Jan. 15, 2014, 12:03 a.m.*
>  *Repository: * kde-baseapps
> Description
>
> Use the QMap iterator instead of foreach(key, map.keys()) in UpdateItemStatesThread::run() and in VersionControlObserver::slotThreadFinished().
>
>   Testing
>
> Works ;)
>
>   Diffs
>
>    - dolphin/src/views/versioncontrol/updateitemstatesthread.cpp (6be07d3)
>    - dolphin/src/views/versioncontrol/versioncontrolobserver.cpp (4d939ee)
>
> View Diff <https://git.reviewboard.kde.org/r/115018/diff/>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20140115/59685aea/attachment.htm>