D25039: Fix Clazy performance issues, const &

[Méven Car]

meven added a comment.


  In D25039#557004 <https://phabricator.kde.org/D25039#557004>, @anthonyfieroni wrote:
  
  > Not using references is not a big problem with QString nor QUrl since they are reference counting objects, say if you don't change their content they are immutable, so
  >
  >   const QString s = other; // it's fine
  >   void func(QString s)
  >   {
  >        const QString o = s; // use o instead of s is also fine, using plain s is fine too, if you don't touch mutability 
  >        ...
  >   }
  >
  
  
  Using cont &, you still save some reference counting overhead and allow the compiler to make potential better optimization.
  In your example you'd also save the boilerplate needed just to have a variable const.
  This also makes API clearer about what to expect.

INLINE COMMENTS

> kossebau wrote in ftp.cpp:1378
> This here makes the code fragile and more confusing. What is the difference between `filename` & `search`? Why is `filename` not const? Can it ever change? Would that be fine for `search` to change as well?
> So while seemingly a correct optimization, as `filename` seems not passed to any method modifying it, the resulting code is very strange to a human reader, the intent behind to have `search` being a const reference to `filename` seems mysterious.
> 
> Without having understood the code, I would simply also make `filename` a const variable, and add a hint why search is a const reference only (hinting this is for optimization).
> Though actually `search` could be possibly be removed and checked what the purpose of the asserts have been and merge this with the code upfront. But outside of scope here, so leaving just a TODO for the next person should be fine. One can still be the next person oneself :)

I believe search can be removed in fact, fileName can be used interchangeably.
That's the const beauty it makes this kind of issue visisble.
But this patch is about fixing atomic warnings, not fixing code around those warnings.
This can be fixed in a subsequent diff, thanks for pointing it out.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D25039

To: meven, #frameworks, dfaure
Cc: anthonyfieroni, kossebau, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20191031/67a7d0e5/attachment-0001.html>