Review Request 127215: simplify code, reduce pointer dereferences

[David Faure]

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



About the unittest failure, I'm available by email to debug that, tests shouldn't fail.
(first thing to check will be: did a ~/.qttest/share/applications/faketextapplication.desktop get created?)


src/sycoca/kbuildmimetypefactory.cpp (line 59)
<https://git.reviewboard.kde.org/r/127215/#comment63346>

    Can you keep the order of the operands? I find
      if (var == constant)
    more readable than the other way around.
    ("is this car green?", not "is green the color of this car?"). In any case that's how it is everywhere in KF5 AFAIK.



src/sycoca/kbuildservicefactory.cpp (line 85)
<https://git.reviewboard.kde.org/r/127215/#comment63347>

    It took me some time to understand the comment -1 +1 = 0, since here there _is_ just a +1. Position after the slash -> +1. No -1 anywhere.
    
    But then I got it - this is about the case where there is no '/', then we get -1 and start from 0. It's obvious to me, while the comment isn't ;)
    So I'd say remove the comment, or make it more explicit ("if no slash, -1 + 1 = 0").



src/sycoca/kbuildservicefactory.cpp (line 96)
<https://git.reviewboard.kde.org/r/127215/#comment63348>

    Shouldn't this one be a QStringLiteral?



src/sycoca/kbuildservicefactory.cpp (line 210)
<https://git.reviewboard.kde.org/r/127215/#comment63349>

    You could have kept the "const" in front of endserv (same below)



src/sycoca/kbuildsycoca.cpp (line 204)
<https://git.reviewboard.kde.org/r/127215/#comment63350>

    what is the clazy warning that encourages to write such a long line of code, rather than the two lines as it was initially?



src/sycoca/ksycocadict.cpp (line 316)
<https://git.reviewboard.kde.org/r/127215/#comment63352>

    I agree.


- David Faure


On Feb. 29, 2016, 12:43 a.m., Nick Shaforostoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127215/
> -----------------------------------------------------------
> 
> (Updated Feb. 29, 2016, 12:43 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kservice
> 
> 
> Description
> -------
> 
> Changes are mostly related to containers/iterators, but there are also few QString related optimizations
> 
> note that i have made a switch from QLinkedList to just QList because it performs better for T=<void*>
> also i have changed QStringList to QSet<QString> in one place to make the code run faster
> 
> 
> Diffs
> -----
> 
>   src/sycoca/kbuildmimetypefactory.cpp 340f76a 
>   src/sycoca/kbuildservicefactory.cpp a74f2e8 
>   src/sycoca/kbuildsycoca.cpp e99e906 
>   src/sycoca/kmimeassociations.cpp 9423b27 
>   src/sycoca/ksycocadict.cpp f33d01a 
>   src/sycoca/ksycocafactory.cpp e410581 
> 
> Diff: https://git.reviewboard.kde.org/r/127215/diff/
> 
> 
> Testing
> -------
> 
> all tests pass, except kmimeassociationstest (fakeApplicationService is NULL), but it fails also without my changes
> 
> 
> Thanks,
> 
> Nick Shaforostoff
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160229/c6765691/attachment-0001.html>