From a67a574b89ae1e12de158d6fdc5b51fdec6f13f3 Mon Sep 17 00:00:00 2001 From: Gianluca Recchia Date: Sun, 28 Oct 2018 12:23:06 +0100 Subject: [PATCH] Reduce function call overhead The arg() function of the QString class has a variable length argument which allows to reduce the number of chained calls to the same function. With proper formatting, readability is not affected. --- src/browser/BrowserService.cpp | 2 +- src/cli/Extract.cpp | 2 +- src/core/Group.cpp | 2 +- src/core/Merger.cpp | 38 ++++++++++------------------ src/gui/AboutDialog.cpp | 6 ++--- src/gui/KMessageWidget.cpp | 8 +++--- src/gui/csvImport/CsvParserModel.cpp | 6 ++--- src/totp/totp.cpp | 6 ++--- 8 files changed, 30 insertions(+), 40 deletions(-) diff --git a/src/browser/BrowserService.cpp b/src/browser/BrowserService.cpp index 456028632..824cc94b6 100644 --- a/src/browser/BrowserService.cpp +++ b/src/browser/BrowserService.cpp @@ -362,7 +362,7 @@ void BrowserService::updateEntry(const QString& id, if (!browserSettings()->alwaysAllowUpdate()) { QMessageBox msgBox; msgBox.setWindowTitle(tr("KeePassXC: Update Entry")); - msgBox.setText(tr("Do you want to update the information in %1 - %2?").arg(QUrl(url).host()).arg(username)); + msgBox.setText(tr("Do you want to update the information in %1 - %2?").arg(QUrl(url).host(), username)); msgBox.setStandardButtons(QMessageBox::Yes); msgBox.addButton(QMessageBox::No); msgBox.setDefaultButton(QMessageBox::No); diff --git a/src/cli/Extract.cpp b/src/cli/Extract.cpp index 32b1bc028..c0b1b1119 100644 --- a/src/cli/Extract.cpp +++ b/src/cli/Extract.cpp @@ -77,7 +77,7 @@ int Extract::execute(const QStringList& arguments) auto fileKey = QSharedPointer::create(); QString errorMsg; if (!fileKey->load(keyFilePath, &errorMsg)) { - err << QObject::tr("Failed to load key file %1: %2").arg(keyFilePath).arg(errorMsg) << endl; + err << QObject::tr("Failed to load key file %1: %2").arg(keyFilePath, errorMsg) << endl; return EXIT_FAILURE; } diff --git a/src/core/Group.cpp b/src/core/Group.cpp index 05cac41e9..72f2490a0 100644 --- a/src/core/Group.cpp +++ b/src/core/Group.cpp @@ -811,7 +811,7 @@ void Group::removeEntry(Entry* entry) { Q_ASSERT_X(m_entries.contains(entry), Q_FUNC_INFO, - QString("Group %1 does not contain %2").arg(this->name()).arg(entry->title()).toLatin1()); + QString("Group %1 does not contain %2").arg(this->name(), entry->title()).toLatin1()); emit entryAboutToRemove(entry); diff --git a/src/core/Merger.cpp b/src/core/Merger.cpp index 9b87a6ac3..2bdff7377 100644 --- a/src/core/Merger.cpp +++ b/src/core/Merger.cpp @@ -95,7 +95,7 @@ Merger::ChangeList Merger::mergeGroup(const MergeContext& context) // Entry is already present in the database. Update it. const bool locationChanged = targetEntry->timeInfo().locationChanged() < sourceEntry->timeInfo().locationChanged(); if (locationChanged && targetEntry->group() != context.m_targetGroup) { - changes << tr("Relocating %1 [%2]").arg(sourceEntry->title()).arg(sourceEntry->uuidToHex()); + changes << tr("Relocating %1 [%2]").arg(sourceEntry->title(), sourceEntry->uuidToHex()); moveEntry(targetEntry, context.m_targetGroup); } changes << resolveEntryConflict(context, sourceEntry, targetEntry); @@ -107,7 +107,7 @@ Merger::ChangeList Merger::mergeGroup(const MergeContext& context) for (Group* sourceChildGroup : sourceChildGroups) { Group* targetChildGroup = context.m_targetRootGroup->findGroupByUuid(sourceChildGroup->uuid()); if (!targetChildGroup) { - changes << tr("Creating missing %1 [%2]").arg(sourceChildGroup->name()).arg(sourceChildGroup->uuidToHex()); + changes << tr("Creating missing %1 [%2]").arg(sourceChildGroup->name(), sourceChildGroup->uuidToHex()); targetChildGroup = sourceChildGroup->clone(Entry::CloneNoFlags, Group::CloneNoFlags); moveGroup(targetChildGroup, context.m_targetGroup); TimeInfo timeinfo = targetChildGroup->timeInfo(); @@ -117,7 +117,7 @@ Merger::ChangeList Merger::mergeGroup(const MergeContext& context) bool locationChanged = targetChildGroup->timeInfo().locationChanged() < sourceChildGroup->timeInfo().locationChanged(); if (locationChanged && targetChildGroup->parent() != context.m_targetGroup) { - changes << tr("Relocating %1 [%2]").arg(sourceChildGroup->name()).arg(sourceChildGroup->uuidToHex()); + changes << tr("Relocating %1 [%2]").arg(sourceChildGroup->name(), sourceChildGroup->uuidToHex()); moveGroup(targetChildGroup, context.m_targetGroup); TimeInfo timeinfo = targetChildGroup->timeInfo(); timeinfo.setLocationChanged(sourceChildGroup->timeInfo().locationChanged()); @@ -146,7 +146,7 @@ Merger::ChangeList Merger::resolveGroupConflict(const MergeContext& context, con // only if the other group is newer, update the existing one. if (timeExisting < timeOther) { - changes << tr("Overwriting %1 [%2]").arg(sourceChildGroup->name()).arg(sourceChildGroup->uuidToHex()); + changes << tr("Overwriting %1 [%2]").arg(sourceChildGroup->name(), sourceChildGroup->uuidToHex()); targetChildGroup->setName(sourceChildGroup->name()); targetChildGroup->setNotes(sourceChildGroup->notes()); if (sourceChildGroup->iconNumber() == 0) { @@ -270,16 +270,12 @@ Merger::ChangeList Merger::resolveEntryConflict_Duplicate(const MergeContext& co Entry* clonedEntry = sourceEntry->clone(Entry::CloneNewUuid | Entry::CloneIncludeHistory); moveEntry(clonedEntry, context.m_targetGroup); markOlderEntry(targetEntry); - changes << tr("Adding backup for older target %1 [%2]") - .arg(targetEntry->title()) - .arg(targetEntry->uuidToHex()); + changes << tr("Adding backup for older target %1 [%2]").arg(targetEntry->title(), targetEntry->uuidToHex()); } else if (comparison > 0) { Entry* clonedEntry = sourceEntry->clone(Entry::CloneNewUuid | Entry::CloneIncludeHistory); moveEntry(clonedEntry, context.m_targetGroup); markOlderEntry(clonedEntry); - changes << tr("Adding backup for older source %1 [%2]") - .arg(sourceEntry->title()) - .arg(sourceEntry->uuidToHex()); + changes << tr("Adding backup for older source %1 [%2]").arg(sourceEntry->title(), sourceEntry->uuidToHex()); } return changes; } @@ -297,8 +293,7 @@ Merger::ChangeList Merger::resolveEntryConflict_KeepLocal(const MergeContext& co // this type of merge changes the database timestamp since reapplying the // old entry is an active change of the database! changes << tr("Reapplying older target entry on top of newer source %1 [%2]") - .arg(targetEntry->title()) - .arg(targetEntry->uuidToHex()); + .arg(targetEntry->title(), targetEntry->uuidToHex()); Entry* agedTargetEntry = targetEntry->clone(Entry::CloneNoFlags); targetEntry->addHistoryItem(agedTargetEntry); } @@ -318,8 +313,7 @@ Merger::ChangeList Merger::resolveEntryConflict_KeepRemote(const MergeContext& c // this type of merge changes the database timestamp since reapplying the // old entry is an active change of the database! changes << tr("Reapplying older source entry on top of newer target %1 [%2]") - .arg(targetEntry->title()) - .arg(targetEntry->uuidToHex()); + .arg(targetEntry->title(), targetEntry->uuidToHex()); targetEntry->beginUpdate(); targetEntry->copyDataFrom(sourceEntry); targetEntry->endUpdate(); @@ -342,9 +336,7 @@ Merger::ChangeList Merger::resolveEntryConflict_MergeHistories(const MergeContex qPrintable(targetEntry->title()), qPrintable(sourceEntry->title()), qPrintable(currentGroup->name())); - changes << tr("Synchronizing from newer source %1 [%2]") - .arg(targetEntry->title()) - .arg(targetEntry->uuidToHex()); + changes << tr("Synchronizing from newer source %1 [%2]").arg(targetEntry->title(), targetEntry->uuidToHex()); moveEntry(clonedEntry, currentGroup); mergeHistory(targetEntry, clonedEntry, mergeMethod); eraseEntry(targetEntry); @@ -355,9 +347,7 @@ Merger::ChangeList Merger::resolveEntryConflict_MergeHistories(const MergeContex qPrintable(targetEntry->group()->name())); const bool changed = mergeHistory(sourceEntry, targetEntry, mergeMethod); if (changed) { - changes << tr("Synchronizing from older source %1 [%2]") - .arg(targetEntry->title()) - .arg(targetEntry->uuidToHex()); + changes << tr("Synchronizing from older source %1 [%2]").arg(targetEntry->title(), targetEntry->uuidToHex()); } } return changes; @@ -550,9 +540,9 @@ Merger::ChangeList Merger::mergeDeletions(const MergeContext& context) } deletions << object; if (entry->group()) { - changes << tr("Deleting child %1 [%2]").arg(entry->title()).arg(entry->uuidToHex()); + changes << tr("Deleting child %1 [%2]").arg(entry->title(), entry->uuidToHex()); } else { - changes << tr("Deleting orphan %1 [%2]").arg(entry->title()).arg(entry->uuidToHex()); + changes << tr("Deleting orphan %1 [%2]").arg(entry->title(), entry->uuidToHex()); } // Entry is inserted into deletedObjects after deletions are processed eraseEntry(entry); @@ -576,9 +566,9 @@ Merger::ChangeList Merger::mergeDeletions(const MergeContext& context) } deletions << object; if (group->parentGroup()) { - changes << tr("Deleting child %1 [%2]").arg(group->name()).arg(group->uuidToHex()); + changes << tr("Deleting child %1 [%2]").arg(group->name(), group->uuidToHex()); } else { - changes << tr("Deleting orphan %1 [%2]").arg(group->name()).arg(group->uuidToHex()); + changes << tr("Deleting orphan %1 [%2]").arg(group->name(), group->uuidToHex()); } eraseGroup(group); } diff --git a/src/gui/AboutDialog.cpp b/src/gui/AboutDialog.cpp index f6a9d15f9..483e4dd2b 100644 --- a/src/gui/AboutDialog.cpp +++ b/src/gui/AboutDialog.cpp @@ -65,9 +65,9 @@ AboutDialog::AboutDialog(QWidget* parent) #endif debugInfo.append("\n").append(QString("%1\n- Qt %2\n- %3\n\n") - .arg(tr("Libraries:")) - .arg(QString::fromLocal8Bit(qVersion())) - .arg(Crypto::backendVersion())); + .arg(tr("Libraries:"), + QString::fromLocal8Bit(qVersion()), + Crypto::backendVersion())); #if QT_VERSION >= QT_VERSION_CHECK(5, 4, 0) debugInfo.append(tr("Operating system: %1\nCPU architecture: %2\nKernel: %3 %4") diff --git a/src/gui/KMessageWidget.cpp b/src/gui/KMessageWidget.cpp index 3d0500708..2e68975e3 100644 --- a/src/gui/KMessageWidget.cpp +++ b/src/gui/KMessageWidget.cpp @@ -309,10 +309,10 @@ void KMessageWidget::setMessageType(KMessageWidget::MessageType type) "}" ".QLabel { color: %6; }" )) - .arg(bg0.name()) - .arg(bg1.name()) - .arg(bg2.name()) - .arg(border.name()) + .arg(bg0.name(), + bg1.name(), + bg2.name(), + border.name()) // DefaultFrameWidth returns the size of the external margin + border width. We know our border is 1px, // so we subtract this from the frame normal QStyle FrameWidth to get our margin .arg(style()->pixelMetric(QStyle::PM_DefaultFrameWidth, nullptr, this) - 1) diff --git a/src/gui/csvImport/CsvParserModel.cpp b/src/gui/csvImport/CsvParserModel.cpp index e69ea1853..8f7d98ec4 100644 --- a/src/gui/csvImport/CsvParserModel.cpp +++ b/src/gui/csvImport/CsvParserModel.cpp @@ -36,9 +36,9 @@ void CsvParserModel::setFilename(const QString& filename) QString CsvParserModel::getFileInfo() { QString a(tr("%1, %2, %3", "file info: bytes, rows, columns") - .arg(tr("%n byte(s)", nullptr, getFileSize())) - .arg(tr("%n row(s)", nullptr, getCsvRows())) - .arg(tr("%n column(s)", nullptr, qMax(0, getCsvCols() - 1)))); + .arg(tr("%n byte(s)", nullptr, getFileSize()), + tr("%n row(s)", nullptr, getCsvRows()), + tr("%n column(s)", nullptr, qMax(0, getCsvCols() - 1)))); return a; } diff --git a/src/totp/totp.cpp b/src/totp/totp.cpp index 4140993c7..bc66fffa4 100644 --- a/src/totp/totp.cpp +++ b/src/totp/totp.cpp @@ -106,9 +106,9 @@ QString Totp::writeSettings(const QSharedPointer settings, const // OTP Url output if (settings->otpUrl || forceOtp) { auto urlstring = QString("otpauth://totp/%1:%2?secret=%3&period=%4&digits=%5&issuer=%1") - .arg(title.isEmpty() ? "KeePassXC" : QString(QUrl::toPercentEncoding(title))) - .arg(username.isEmpty() ? "none" : QString(QUrl::toPercentEncoding(username))) - .arg(QString(Base32::sanitizeInput(settings->key.toLatin1()))) + .arg(title.isEmpty() ? "KeePassXC" : QString(QUrl::toPercentEncoding(title)), + username.isEmpty() ? "none" : QString(QUrl::toPercentEncoding(username)), + QString(Base32::sanitizeInput(settings->key.toLatin1()))) .arg(settings->step) .arg(settings->digits);