Review Request 112248: 313504: prefer QLatin1String

Mark Kretschmann kretschmann at kde.org
Sun Aug 25 08:24:10 UTC 2013


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


Sorry, I am against this one because I think it considerably deteriorates code readability for very little performance gain.

Frank, most of the checks on EBN are for micro-optimizations. They are low hanging fruit, and as such they generally make sense to follow, but there are corner cases like this one where the cost outweighs the benefits.

- Mark Kretschmann


On Aug. 24, 2013, 5:32 p.m., Frank Meerkoetter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112248/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2013, 5:32 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> -------
> 
> QLatin1String is more efficient.
> 
> http://www.englishbreakfastnetwork.org/krazy/reports/extragear/multimedia/amarok/index.html
> [...]
> 33: Check for strings used improperly or should be i18n. [strings]...OOPS! 78 issues found!
> [...]
> 
> 
> This addresses bug 313504.
>     https://bugs.kde.org/show_bug.cgi?id=313504
> 
> 
> Diffs
> -----
> 
>   shared/tag_helpers/TagHelper.cpp 7528a77 
>   src/MediaDeviceCache.cpp 73e8f34 
>   src/ScriptManager.cpp 4b4fc14 
>   src/amarokurls/NavigationUrlGenerator.cpp 0456976 
>   src/amarokurls/NavigationUrlRunner.cpp bde18e5 
>   src/context/applets/info/InfoApplet.cpp 760b241 
>   src/context/engines/similarartists/SimilarArtistsEngine.cpp 898beb4 
>   src/context/engines/tabs/TabsEngine.cpp 24a89df 
>   src/core-impl/collections/audiocd/AudioCdCollection.cpp 3dfa7c3 
>   src/core-impl/collections/daap/DaapCollection.cpp 20e7b4f 
>   src/core-impl/collections/db/sql/SqlMeta.cpp b399ed3 
>   src/core-impl/collections/ipodcollection/IpodCollectionFactory.cpp 7ecdcfa 
>   src/core-impl/collections/support/ArtistHelper.cpp 4c3f742 
>   src/core-impl/collections/umscollection/UmsCollection.cpp ad38461 
>   src/core-impl/collections/upnpcollection/UpnpBrowseCollection.cpp 6467964 
>   src/core-impl/collections/upnpcollection/UpnpCollectionFactory.cpp 80ea8de 
>   src/core-impl/collections/upnpcollection/UpnpMeta.cpp 9b660de 
>   src/core-impl/meta/cue/CueFileSupport.cpp 77d27e6 
>   src/core-impl/meta/file/File.cpp f2abea4 
>   src/core-impl/playlists/types/file/m3u/M3UPlaylist.cpp 28e3b00 
>   src/core/support/Amarok.cpp ca59725 
>   src/dialogs/CollectionSetup.cpp 0672421 
>   src/services/amazon/AmazonStore.cpp eb42566 
>   src/services/ampache/AmpacheAccountLogin.cpp 26aebed 
>   src/services/ampache/AmpacheServiceQueryMaker.cpp b94c3b0 
>   src/services/lastfm/ScrobblerAdapter.cpp 3c81546 
>   src/services/magnatune/MagnatuneSqlCollection.cpp 2f50e90 
>   src/services/mp3tunes/Mp3tunesWorkers.cpp a7ebd36 
>   src/statsyncing/collection/CollectionProvider.cpp a26cec6 
>   src/widgets/Osd.cpp 32c46e9 
>   supplementary_scripts/licenseChecker/log.cpp ba9b0fd 
>   supplementary_scripts/licenseChecker/main.cpp 4e6bba8 
>   tests/core-impl/collections/db/sql/TestSqlCollectionLocation.cpp ab41db3 
>   tests/core-impl/support/TestTrackLoader.cpp ce369e2 
>   utilities/afttagger/AFTTagger.cpp 5a60c6b 
>   utilities/amzdownloader/AmzDownloader.cpp e6771de 
>   utilities/collectionscanner/CollectionScanner.cpp dd4ae37 
> 
> Diff: http://git.reviewboard.kde.org/r/112248/diff/
> 
> 
> Testing
> -------
> 
> Unit tests pass.
> 
> 
> Thanks,
> 
> Frank Meerkoetter
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20130825/9116cd8d/attachment.html>


More information about the Amarok-devel mailing list