[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-edu-devel
Subject:    Re: [Marble-devel] Review Request: fixed foreach loops code checker issues
From:       "Jens-Michael Hoffmann" <jmho () jmho ! de>
Date:       2013-01-06 21:22:45
Message-ID: 201301062222.45470.jmho () jmho ! de
[Download RAW message or body]

Am Sonntag, 6. Januar 2013, 17:40:11 schrieb Kevin Krammer:
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108217/#review24845
> -----------------------------------------------------------
> 
> 
> Consider this review more as a hint for how to handle this in applications
> where the developers care about performance of such loops. Obviously the
> developers here are fine with incurring lookups on each iteration
> otherwise they would not have used foreach on keys() or values() in the
> first place
> 
> 
> src/lib/FileManager.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19068>
> 
>     const_iterator, constBegin
> 
> 
> 
> src/lib/FileManager.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19069>
> 
>     const_iterator, constBegin
> 
> 
> 
> src/lib/FileManager.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19064>
> 
>     no, this is a lookup, the iterator has a value() method that returns
> the value the iterator points to
> 
> 
> 
> src/lib/StackedTileLoader.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19065>
> 
>     const_iterator, constBegin
> 
> 
> 
> src/lib/StackedTileLoader.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19066>
> 
>     const_iterator, constEnd
> 
> 
> 
> src/lib/routing/RouteAnnotator.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19070>
> 
>     const_iterator, constBegin
> 
> 
> 
> src/lib/routing/RouteAnnotator.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19071>
> 
>     const_iterator, constEnd
> 
> 
> 
> src/lib/routing/RouteAnnotator.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19072>
> 
>     no, this is an unneeded lookup
>     itpoint.value() does the same more efficiently
> 
> 
> 
> src/plugins/render/opencaching/OpenCachingModel.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19073>
> 
>     const_iterator, constBegin
> 
> 
> 
> src/plugins/render/opencaching/OpenCachingModel.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19074>
> 
>     const_iterator, constEnd
> 
> 
> 
> src/plugins/render/opencaching/OpenCachingModel.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19075>
> 
>     unneeded lookup overhead, use itpoint.value() instead
> 
> 
> 
> src/plugins/render/opencaching/OpenCachingModel.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19076>
> 
>     unneeded lookup overhead, use itpoint.value() instead
> 
> 
> 
> src/plugins/render/opencaching/OpenCachingModel.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19077>
> 
>     unneeded lookup overhead, use itpoint.value() instead
> 
> 
> 
> src/plugins/render/opencaching/OpenCachingModel.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19078>
> 
>     unneeded lookup overhead, use itpoint.value() instead
> 
> 
> 
> src/plugins/render/opencaching/OpenCachingModel.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19079>
> 
>     unneeded lookup overhead, use itpoint.value() instead
> 
> 
> 
> tests/RenderPluginTest.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19080>
> 
>     const_iterator, constBegin
> 
> 
> 
> tests/RenderPluginTest.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19081>
> 
>     const_iterator, constEnd
> 
> 
> 
> tests/RenderPluginTest.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19082>
> 
>     unneeded lookup overhead at expected->settings().value(), use
> itpoint.value() instead
> 
> 
> 
> tests/TestGeoDataWriter.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19083>
> 
>     const_iterator, constBegin
> 
> 
> 
> tests/TestGeoDataWriter.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19084>
> 
>     const_iterator, constEnd
> 
> 
> 
> tests/TestGeoSceneWriter.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19085>
> 
>     const_iterator, constBegin
> 
> 
> 
> tests/TestGeoSceneWriter.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19086>
> 
>     const_iterator, constEnd
> 
> 
> 
> tests/TestGeoSceneWriter.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19087>
> 
>     const_iterator, constBegin
> 
> 
> 
> tests/TestGeoSceneWriter.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19088>
> 
>     const_iterator, constEnd
> 
> 
> 
> tests/TestGeoSceneWriter.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19089>
> 
>     const_iterator, constBegin
> 
> 
> 
> tests/TestGeoSceneWriter.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19090>
> 
>     const_iterator, constEnd
> 
> 
> 
> tools/osm-addresses/OsmParser.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19091>
> 
>     const_iterator, constBegin
> 
> 
> 
> tools/osm-addresses/OsmParser.cpp
> <http://git.reviewboard.kde.org/r/108217/#comment19092>
> 
>     const_iterator, constEnd
> 
> 
> - Kevin Krammer
> 

Thanks for the thorough review, it is very much appreciated. These are indeed 
important points.


kind regards,
Jens-Michael

_______________________________________________
kde-edu mailing list
kde-edu@mail.kde.org
https://mail.kde.org/mailman/listinfo/kde-edu
[prev in list] [next in list] [prev in thread] [next in thread]