D25039: Fix Clazy performance issues, const &

Friedrich W. H. Kossebau noreply at phabricator.kde.org
Thu Oct 31 14:50:48 GMT 2019


kossebau added inline comments.

INLINE COMMENTS

> ftp.cpp:1376
>      QString parentDir;
> -    QString filename = tempurl.fileName();
> +    const QString &filename = tempurl.fileName();
>      Q_ASSERT(!filename.isEmpty());

`QUrl::fileName()` returns a value QString, so just

  const QString filename = empurl.fileName();

While

  const QString &filename = empurl.fileName();

also is fine code IIRC, as I once learned to my surprise,  as the `const QString &` here means to tell the compiler the actual value instance should be hold only until the last use of the variable, not the end of the scope in which the variable exists, this seems also not well known by others, so might only make them confuse.
Given this is an optimization not needed here, IMHO no need to use this technique here.

> kossebau wrote in kcookiejar.cpp:1103
> This here seems some overleft from when the method actually needed a modfyable copy of _domain. Seems this in no longer the case.
> So we can just rename `_domain` to `domain` in the arg list instead, and be done.

Hm, why the change of all _domain to domain, not the other way around?
The _xyz naming of argument variables has been seen usually when it was needed to create a copy of the argument in the implementation, because e.g. the const-refness was in the way. As the normal naming of arguments and variables is without the _ prefix,. The prefix is then used with the argument name to denote this is the input to the actual variable then used in the implementation.

So:

  void foo(const Type arg)
   {
      // need to do non-const things with arg value, meh
  }

->

  void foo(const Type _arg)
   {
      Type arg(_arg);
      // do non-const things with arg value
  }

> kdirmodel.cpp:495
>          if (!urlsBeingFetched.isEmpty()) {
> -            const QUrl dirUrl(url);
> +            const QUrl &dirUrl(url);
>              for (const QUrl &urlFetched : qAsConst(urlsBeingFetched)) {

`const QUrl &dirUrl = url;` for consistency, please.

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/734babc5/attachment.html>


More information about the Kde-frameworks-devel mailing list