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