Cleanup

1. Add a checkArgumentCount() helper method.
2. Fix a plural form of the check method.
3. Remove repeating translation for every UpdateOperation subclass.
4. Use always QList.at() for checking arguments,
   instead of random usage of first(), last() and at().
5. Fix const corectness.
6. Move isPersistant into #ifdef Q_OS_WIN inside
   EnvironmentVariableOperation.

Change-Id: Idaa12265f1816871de0a4ea3ae586512859a04e3
Reviewed-by: Tim Jenssen <tim.jenssen@theqtcompany.com>
Reviewed-by: Jarek Kobus <jaroslaw.kobus@theqtcompany.com>

15 files changed, 43 insertions, 104 deletions

diff --git a/src/libs/installer/consumeoutputoperation.cpp b/src/libs/installer/consumeoutputoperation.cpp
index 00b396ad6..542626553 100644
--- a/src/libs/installer/consumeoutputoperation.cpp
+++ b/src/libs/installer/consumeoutputoperation.cpp
@@ -59,13 +59,10 @@ bool ConsumeOutputOperation::performOperation()
     // 2. executable path
     // 3. argument for the executable
     // 4. more arguments possible ...
-    if (arguments().count() < 3) {
-        setError(InvalidArguments);
-        setErrorString(tr("Invalid arguments in %0: %1 arguments given, %2 expected%3.").arg(name()).arg(
-            arguments().count()).arg(tr("at least 2"), QLatin1String("(<to be saved installer key name>, "
-            "<executable>, [argument1], [argument2], ...)")));
+
+    if (!checkArgumentCount(2, INT_MAX, tr("(<to be saved installer key name>, "
+                                      "<executable>, [argument1], [argument2], ...)")))
         return false;
-    }
 
     PackageManagerCore *const core = value(QLatin1String("installer")).value<PackageManagerCore*>();
     if (!core) {
diff --git a/src/libs/installer/copydirectoryoperation.cpp b/src/libs/installer/copydirectoryoperation.cpp
index cd2b1b341..d94a5849f 100644
--- a/src/libs/installer/copydirectoryoperation.cpp
+++ b/src/libs/installer/copydirectoryoperation.cpp
@@ -63,13 +63,10 @@ void CopyDirectoryOperation::backup()
 
 bool CopyDirectoryOperation::performOperation()
 {
-    const QStringList args = arguments();
-    if (args.count() < 2 || args.count() > 3) {
-        setError(InvalidArguments);
-        setErrorString(tr("Invalid arguments in %0: %1 arguments given, %2 expected%3.")
-            .arg(name()).arg(arguments().count()).arg(tr("2 or 3"), tr(" (<source> <target> [forceOverwrite])")));
+    if (!checkArgumentCount(2, 3, tr(" (<source> <target> [forceOverwrite])")))
         return false;
-    }
+
+    const QStringList args = arguments();
     const QString sourcePath = args.at(0);
     const QString targetPath = args.at(1);
     bool overwrite = false;
diff --git a/src/libs/installer/createdesktopentryoperation.cpp b/src/libs/installer/createdesktopentryoperation.cpp
index 11e977d08..8d71c2c07 100644
--- a/src/libs/installer/createdesktopentryoperation.cpp
+++ b/src/libs/installer/createdesktopentryoperation.cpp
@@ -127,16 +127,11 @@ void CreateDesktopEntryOperation::backup()
 
 bool CreateDesktopEntryOperation::performOperation()
 {
-    const QStringList args = arguments();
-    if (args.count() != 2) {
-        setError(InvalidArguments);
-        setErrorString(tr("Invalid arguments in %0: %1 arguments given, %2 expected%3.")
-            .arg(name()).arg(arguments().count()).arg(tr("exactly 2"), QLatin1String("")));
+    if (!checkArgumentCount(2))
         return false;
-    }
 
     const QString filename = absoluteFileName();
-    const QString &values = args[1];
+    const QString &values = arguments().at(1);
 
     QFile file(filename);
     if (file.exists() && !file.remove()) {
diff --git a/src/libs/installer/createlinkoperation.cpp b/src/libs/installer/createlinkoperation.cpp
index 6117ef602..fc0950870 100644
--- a/src/libs/installer/createlinkoperation.cpp
+++ b/src/libs/installer/createlinkoperation.cpp
@@ -50,15 +50,10 @@ void CreateLinkOperation::backup()
 
 bool CreateLinkOperation::performOperation()
 {
-    QStringList args = arguments();
-
-    if (args.count() != 2) {
-        setError(InvalidArguments);
-        setErrorString(tr("Invalid arguments in %0: %1 arguments given, %2 expected%3.")
-            .arg(name()).arg(arguments().count()).arg(tr("exactly 2"), QLatin1String("")));
+    if (!checkArgumentCount(2))
         return false;
-    }
 
+    const QStringList args = arguments();
     const QString& linkPath = args.at(0);
     const QString& targetPath = args.at(1);
     Link link = Link::create(linkPath, targetPath);
diff --git a/src/libs/installer/createlocalrepositoryoperation.cpp b/src/libs/installer/createlocalrepositoryoperation.cpp
index 55453a916..0904ab8dd 100644
--- a/src/libs/installer/createlocalrepositoryoperation.cpp
+++ b/src/libs/installer/createlocalrepositoryoperation.cpp
@@ -152,16 +152,12 @@ bool CreateLocalRepositoryOperation::performOperation()
     AutoHelper helper(this);
     emit progressChanged(0.0);
 
-    const QStringList args = arguments();
-
-    if (args.count() != 2) {
-        setError(InvalidArguments);
-        setErrorString(tr("Invalid arguments in %0: %1 arguments given, %2 expected%3.")
-            .arg(name()).arg(arguments().count()).arg(tr("exactly 2"), QLatin1String("")));
+    if (!checkArgumentCount(2))
         return false;
-    }
 
     try {
+        const QStringList args = arguments();
+
         const QString binaryPath = QFileInfo(args.at(0)).absoluteFilePath();
         // Note the "/" at the end, important to make copy directory operation behave well
         const QString repoPath = QFileInfo(args.at(1)).absoluteFilePath() + QLatin1Char('/');
diff --git a/src/libs/installer/createshortcutoperation.cpp b/src/libs/installer/createshortcutoperation.cpp
index 060672bd8..2ee7dfbf4 100644
--- a/src/libs/installer/createshortcutoperation.cpp
+++ b/src/libs/installer/createshortcutoperation.cpp
@@ -179,23 +179,18 @@ void CreateShortcutOperation::backup()
 
 bool CreateShortcutOperation::performOperation()
 {
+    if (!checkArgumentCount(2, 3, tr("(optional: \"workingDirectory=...\", \"iconPath=...\", \"iconId=...\")")))
+        return false;
+
     QStringList args = arguments();
 
     const QString iconId = takeArgument(QString::fromLatin1("iconId="), &args);
     const QString iconPath = takeArgument(QString::fromLatin1("iconPath="), &args);
     const QString workingDir = takeArgument(QString::fromLatin1("workingDirectory="), &args);
 
-    if (args.count() != 2 && args.count() != 3) {
-        setError(InvalidArguments);
-        setErrorString(tr("Invalid arguments in %0: %1 arguments given, %2 expected%3.")
-            .arg(name()).arg(arguments().count()).arg(tr("2 or 3"),
-            tr(" (optional: 'workingDirectory=...', 'iconPath=...', 'iconId=...')")));
-        return false;
-    }
-
     const QString linkTarget = args.at(0);
     const QString linkLocation = args.at(1);
-    const QString targetArguments = args.value(2); //used value because it could be not existing
+    const QString targetArguments = args.value(2);  // value() used since it's optional
 
     const QString linkPath = QFileInfo(linkLocation).absolutePath().trimmed();
     const bool created = QDir(linkPath).exists() || QDir::root().mkpath(linkPath);
diff --git a/src/libs/installer/elevatedexecuteoperation.cpp b/src/libs/installer/elevatedexecuteoperation.cpp
index 3e766f801..1f439067c 100644
--- a/src/libs/installer/elevatedexecuteoperation.cpp
+++ b/src/libs/installer/elevatedexecuteoperation.cpp
@@ -80,12 +80,9 @@ bool ElevatedExecuteOperation::performOperation()
 {
     // This operation receives only one argument. It is the complete
     // command line of the external program to execute.
-    if (arguments().isEmpty()) {
-        setError(InvalidArguments);
-        setErrorString(tr("Invalid arguments in %0: %1 arguments given, %2 expected%3.")
-            .arg(name()).arg(arguments().count()).arg(tr("at least 1"), QLatin1String("")));
+    if (!checkArgumentCount(1, INT_MAX))
         return false;
-    }
+
     QStringList args;
     foreach (const QString &argument, arguments()) {
         if (argument!=QLatin1String("UNDOEXECUTE"))
diff --git a/src/libs/installer/environmentvariablesoperation.cpp b/src/libs/installer/environmentvariablesoperation.cpp
index f41b90893..55332f540 100644
--- a/src/libs/installer/environmentvariablesoperation.cpp
+++ b/src/libs/installer/environmentvariablesoperation.cpp
@@ -121,21 +121,16 @@ UpdateOperation::Error undoSetting(const QString &regPath,
 
 bool EnvironmentVariableOperation::performOperation()
 {
-    QStringList args = arguments();
-    if (args.count() < 2 || args.count() > 4) {
-        setError(InvalidArguments);
-        setErrorString(tr("Invalid arguments in %0: %1 arguments given, %2 expected%3.")
-            .arg(name()).arg(arguments().count()).arg(tr("2 to 4"), QLatin1String("")));
+    if (!checkArgumentCount(2, 4))
         return false;
-    }
 
-    const QString name = arguments().at(0);
-    const QString value = arguments().at(1);
-    bool isPersistent = false;
+    const QStringList args = arguments();
+    const QString name = args.at(0);
+    const QString value = args.at(1);
 
 #ifdef Q_OS_WIN
-    isPersistent = arguments().count() >= 3 ? arguments().at(2) == QLatin1String("true") : true;
-    const bool isSystemWide = arguments().count() >= 4 ? arguments().at(3) == QLatin1String("true") : false;
+    const bool isPersistent = arguments().count() > 2 ? arguments().at(2) == QLatin1String("true") : true;
+    const bool isSystemWide = arguments().count() > 3 ? arguments().at(3) == QLatin1String("true") : false;
     QString oldvalue;
     if (isPersistent) {
         const QString regPath = isSystemWide ? QLatin1String("HKEY_LOCAL_MACHINE\\SYSTEM\\CurrentControlSet"
@@ -158,9 +153,8 @@ bool EnvironmentVariableOperation::performOperation()
         setValue(QLatin1String("oldvalue"), oldvalue);
         return true;
     }
-#endif
     Q_ASSERT(!isPersistent);
-    Q_UNUSED(isPersistent)
+#endif
 
     setValue(QLatin1String("oldvalue"), Environment::instance().value(name));
     Environment::instance().setTemporaryValue(name, value);
diff --git a/src/libs/installer/extractarchiveoperation.cpp b/src/libs/installer/extractarchiveoperation.cpp
index dba695593..d75913d7e 100644
--- a/src/libs/installer/extractarchiveoperation.cpp
+++ b/src/libs/installer/extractarchiveoperation.cpp
@@ -54,15 +54,11 @@ void ExtractArchiveOperation::backup()
 
 bool ExtractArchiveOperation::performOperation()
 {
-    const QStringList args = arguments();
-    if (args.count() != 2) {
-        setError(InvalidArguments);
-        setErrorString(tr("Invalid arguments in %0: %1 arguments given, %2 expected%3.")
-            .arg(name()).arg(arguments().count()).arg(tr("exactly 2"), QLatin1String("")));
+    if (!checkArgumentCount(2))
         return false;
-    }
 
-    const QString archivePath = args.first();
+    const QStringList args = arguments();
+    const QString archivePath = args.at(0);
     const QString targetDir = args.at(1);
 
     Receiver receiver;
diff --git a/src/libs/installer/fakestopprocessforupdateoperation.cpp b/src/libs/installer/fakestopprocessforupdateoperation.cpp
index d30aa370e..562816bfc 100644
--- a/src/libs/installer/fakestopprocessforupdateoperation.cpp
+++ b/src/libs/installer/fakestopprocessforupdateoperation.cpp
@@ -57,11 +57,8 @@ bool FakeStopProcessForUpdateOperation::performOperation()
 bool FakeStopProcessForUpdateOperation::undoOperation()
 {
     setError(KDUpdater::UpdateOperation::NoError);
-    if (arguments().size() != 1) {
-        setError(KDUpdater::UpdateOperation::InvalidArguments, tr("Number of arguments does not "
-            "match: one is required"));
+    if (!checkArgumentCount(1))
         return false;
-    }
 
     PackageManagerCore *const core = value(QLatin1String("installer")).value<PackageManagerCore*>();
     if (!core) {
diff --git a/src/libs/installer/globalsettingsoperation.cpp b/src/libs/installer/globalsettingsoperation.cpp
index 3623487a3..f8d5ee439 100644
--- a/src/libs/installer/globalsettingsoperation.cpp
+++ b/src/libs/installer/globalsettingsoperation.cpp
@@ -105,17 +105,13 @@ Operation *GlobalSettingsOperation::clone() const
 
 QSettingsWrapper *GlobalSettingsOperation::setup(QString *key, QString *value, const QStringList &arguments)
 {
-    if (arguments.count() != 3 && arguments.count() != 4 && arguments.count() != 5) {
-        setError(InvalidArguments);
-        setErrorString(tr("Invalid arguments in %0: %1 arguments given, %2 expected%3.")
-            .arg(name()).arg(arguments.count()).arg(tr("3, 4 or 5"), QLatin1String("")));
+    if (!checkArgumentCount(3, 5))
         return 0;
-    }
 
     if (arguments.count() == 5) {
         QSettingsWrapper::Scope scope = QSettingsWrapper::UserScope;
         if (arguments.at(0) == QLatin1String("SystemScope"))
-          scope = QSettingsWrapper::SystemScope;
+            scope = QSettingsWrapper::SystemScope;
         const QString &company = arguments.at(1);
         const QString &application = arguments.at(2);
         *key = arguments.at(3);
diff --git a/src/libs/installer/installiconsoperation.cpp b/src/libs/installer/installiconsoperation.cpp
index f957623bb..7d5bf08ce 100644
--- a/src/libs/installer/installiconsoperation.cpp
+++ b/src/libs/installer/installiconsoperation.cpp
@@ -109,16 +109,12 @@ void InstallIconsOperation::backup()
 
 bool InstallIconsOperation::performOperation()
 {
-    const QStringList args = arguments();
-    if ((args.count() != 1) && (args.count() != 2)) {
-        setError(InvalidArguments);
-        setErrorString(tr("Invalid arguments in %0: %1 arguments given, %2 expected%3.")
-            .arg(name()).arg(arguments().count()).arg(tr("1 or 2"), tr(" (Sourcepath, [Vendorprefix])")));
+    if (!checkArgumentCount(1, 2, tr("(Sourcepath, [Vendorprefix])")))
         return false;
-    }
 
+    const QStringList args = arguments();
     const QString source = args.at(0);
-    const QString vendor = args.value(1);
+    const QString vendor = args.value(1); // value() used since it's optional
 
     if (source.isEmpty()) {
         setError(InvalidArguments);
diff --git a/src/libs/installer/linereplaceoperation.cpp b/src/libs/installer/linereplaceoperation.cpp
index 9063f8848..c84d1922c 100644
--- a/src/libs/installer/linereplaceoperation.cpp
+++ b/src/libs/installer/linereplaceoperation.cpp
@@ -51,18 +51,14 @@ void LineReplaceOperation::backup()
 
 bool LineReplaceOperation::performOperation()
 {
-    const QStringList args = arguments();
-
     // Arguments:
     // 1. filename
     // 2. startsWith Search-String
     // 3. Replace-Line-String
-    if (args.count() != 3) {
-        setError(InvalidArguments);
-        setErrorString(tr("Invalid arguments in %0: %1 arguments given, %2 expected%3.")
-            .arg(name()).arg(arguments().count()).arg(tr("exactly 3"), QLatin1String("")));
+    if (!checkArgumentCount(3))
         return false;
-    }
+
+    const QStringList args = arguments();
     const QString fileName = args.at(0);
     const QString searchString = args.at(1);
     const QString replaceString = args.at(2);
diff --git a/src/libs/installer/replaceoperation.cpp b/src/libs/installer/replaceoperation.cpp
index c2aec65a8..98a0e7bc9 100644
--- a/src/libs/installer/replaceoperation.cpp
+++ b/src/libs/installer/replaceoperation.cpp
@@ -51,18 +51,14 @@ void ReplaceOperation::backup()
 
 bool ReplaceOperation::performOperation()
 {
-    const QStringList args = arguments();
-
     // Arguments:
     // 1. filename
     // 2. Source-String
     // 3. Replace-String
-    if (args.count() != 3) {
-        setError(InvalidArguments);
-        setErrorString(tr("Invalid arguments in %0: %1 arguments given, %2 expected%3.")
-            .arg(name()).arg(arguments().count()).arg(tr("exactly 3"), QLatin1String("")));
+    if (!checkArgumentCount(3))
         return false;
-    }
+
+    const QStringList args = arguments();
     const QString fileName = args.at(0);
     const QString before = args.at(1);
     const QString after = args.at(2);
diff --git a/src/libs/installer/simplemovefileoperation.cpp b/src/libs/installer/simplemovefileoperation.cpp
index 5edb392e1..54a5583ee 100644
--- a/src/libs/installer/simplemovefileoperation.cpp
+++ b/src/libs/installer/simplemovefileoperation.cpp
@@ -49,14 +49,10 @@ void SimpleMoveFileOperation::backup()
 
 bool SimpleMoveFileOperation::performOperation()
 {
-    const QStringList args = arguments();
-    if (args.count() != 2) {
-        setError(InvalidArguments);
-        setErrorString(tr("Invalid arguments in %0: %1 arguments given, %2 expected%3.")
-            .arg(name()).arg(arguments().count()).arg(tr("exactly 2"), QLatin1String("")));
+    if (!checkArgumentCount(2))
         return false;
-    }
 
+    const QStringList args = arguments();
     const QString source = args.at(0);
     const QString target = args.at(1);