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>

3 files changed, 62 insertions, 47 deletions

diff --git a/src/libs/kdtools/kdupdaterupdateoperation.cpp b/src/libs/kdtools/kdupdaterupdateoperation.cpp
index 8b6cbec64..05c0995ac 100644
--- a/src/libs/kdtools/kdupdaterupdateoperation.cpp
+++ b/src/libs/kdtools/kdupdaterupdateoperation.cpp
@@ -180,6 +180,42 @@ QStringList UpdateOperation::arguments() const
     return m_arguments;
 }
 
+bool UpdateOperation::checkArgumentCount(int minArgCount, int maxArgCount,
+                                         const QString &argDescription)
+{
+    const int argCount = arguments().count();
+    if (argCount < minArgCount || argCount > maxArgCount) {
+        setError(InvalidArguments);
+        QString countRange;
+        if (minArgCount == maxArgCount)
+            countRange = tr("exactly %1").arg(minArgCount);
+        else if (maxArgCount == INT_MAX)
+            countRange = tr("at least %1").arg(minArgCount);
+        else if (minArgCount == 0)
+            countRange = tr("not more than %1").arg(maxArgCount);
+        else if (minArgCount == maxArgCount - 1)
+            countRange = tr("%1 or %2").arg(minArgCount).arg(maxArgCount);
+        else
+            countRange = tr("%1 to %2").arg(minArgCount).arg(maxArgCount);
+
+        if (argDescription.isEmpty())
+            setErrorString(tr("Invalid arguments in %1: %n arguments given, "
+                              "%2 arguments expected.", 0, argCount)
+                           .arg(name(), countRange));
+        else
+            setErrorString(tr("Invalid arguments in %1: %n arguments given, "
+                              "%2 arguments expected in the form: %3.", 0, argCount)
+                           .arg(name(), countRange, argDescription));
+        return false;
+    }
+    return true;
+}
+
+bool UpdateOperation::checkArgumentCount(int argCount)
+{
+    return checkArgumentCount(argCount, argCount);
+}
+
 struct StartsWith
 {
     StartsWith(const QString &searchTerm)
diff --git a/src/libs/kdtools/kdupdaterupdateoperation.h b/src/libs/kdtools/kdupdaterupdateoperation.h
index d841fb564..7707a2d62 100644
--- a/src/libs/kdtools/kdupdaterupdateoperation.h
+++ b/src/libs/kdtools/kdupdaterupdateoperation.h
@@ -90,6 +90,8 @@ protected:
     void setError(int error, const QString &errorString = QString());
     void registerForDelayedDeletion(const QStringList &files);
     bool deleteFileNowOrLater(const QString &file, QString *errorString = 0);
+    bool checkArgumentCount(int minArgCount, int maxArgCount, const QString &argDescription = QString());
+    bool checkArgumentCount(int argCount);
 
 private:
     QString m_name;
diff --git a/src/libs/kdtools/kdupdaterupdateoperations.cpp b/src/libs/kdtools/kdupdaterupdateoperations.cpp
index 31d9d1f44..90dbc7d60 100644
--- a/src/libs/kdtools/kdupdaterupdateoperations.cpp
+++ b/src/libs/kdtools/kdupdaterupdateoperations.cpp
@@ -146,11 +146,8 @@ bool CopyOperation::performOperation()
 {
     // We need two args to complete the copy operation. First arg provides the complete file name of source
     // Second arg provides the complete file name of dest
-    if (arguments().count() != 2) {
-        setError(InvalidArguments);
-        setErrorString(tr("Invalid arguments: %1 arguments given, 2 expected.").arg(arguments().count()));
+    if (!checkArgumentCount(2))
         return false;
-    }
 
     QString source = sourcePath();
     QString destination = destinationPath();
@@ -273,14 +270,11 @@ bool MoveOperation::performOperation()
 {
     // We need two args to complete the copy operation. // First arg provides the complete file name of
     // source, second arg provides the complete file name of dest
-    const QStringList args = this->arguments();
-    if (args.count() != 2) {
-        setError(InvalidArguments);
-        setErrorString(tr("Invalid arguments: %1 arguments given, 2 expected.").arg(args.count()));
+    if (!checkArgumentCount(2))
         return false;
-    }
 
-    const QString dest = args.last();
+    const QStringList args = arguments();
+    const QString dest = args.at(1);
     // If destination file exists, then we cannot use QFile::copy() because it does not overwrite an existing
     // file. So we remove the destination file.
     if (QFile::exists(dest)) {
@@ -293,7 +287,7 @@ bool MoveOperation::performOperation()
     }
 
     // Copy source to destination.
-    QFile file(args.first());
+    QFile file(args.at(0));
     if (!file.copy(dest)) {
         setError(UserDefinedError);
         setErrorString(tr("Could not copy %1 to %2: %3").arg(file.fileName(), dest, file.errorString()));
@@ -372,13 +366,10 @@ void DeleteOperation::backup()
 bool DeleteOperation::performOperation()
 {
     // Requires only one parameter. That is the name of the file to remove.
-    const QStringList args = this->arguments();
-    if (args.count() != 1) {
-        setError(InvalidArguments);
-        setErrorString(tr("Invalid arguments: %1 arguments given, 1 expected.").arg(args.count()));
+    if (!checkArgumentCount(1))
         return false;
-    }
-    return deleteFileNowOrLater(args.first());
+
+    return deleteFileNowOrLater(arguments().at(0));
 }
 
 bool DeleteOperation::undoOperation()
@@ -459,14 +450,10 @@ void MkdirOperation::backup()
 bool MkdirOperation::performOperation()
 {
     // Requires only one parameter. That is the path which should be created
-    QStringList args = this->arguments();
-    if (args.count() != 1) {
-        setError(InvalidArguments);
-        setErrorString(tr("Invalid arguments: %1 arguments given, 1 expected.").arg(args.count()));
+    if (!checkArgumentCount(1))
         return false;
-    }
 
-    const QString dirName = args.first();
+    const QString dirName = arguments().at(0);
     const bool created = QDir::root().mkpath(dirName);
     if (!created) {
         setError(UserDefinedError);
@@ -538,26 +525,23 @@ void RmdirOperation::backup()
 bool RmdirOperation::performOperation()
 {
     // Requires only one parameter. That is the name of the file to remove.
-    const QStringList args = this->arguments();
-    if (args.count() != 1) {
-        setError(InvalidArguments);
-        setErrorString(tr("Invalid arguments: %1 arguments given, 1 expected.").arg(args.count()));
+    if (!checkArgumentCount(1))
         return false;
-    }
 
-    QDir dir(args.first());
+    const QString firstArg = arguments().at(0);
+    QDir dir(firstArg);
     if (!dir.exists()) {
         setError(UserDefinedError);
-        setErrorString(tr("Could not remove folder %1: The folder does not exist.").arg(args.first()));
+        setErrorString(tr("Could not remove folder %1: The folder does not exist.").arg(firstArg));
         return false;
     }
 
     errno = 0;
-    const bool removed = dir.rmdir(args.first());
+    const bool removed = dir.rmdir(firstArg);
     setValue(QLatin1String("removed"), removed);
     if (!removed) {
         setError(UserDefinedError);
-        setErrorString(tr("Could not remove folder %1: %2").arg(args.first(), errnoToQString(errno)));
+        setErrorString(tr("Could not remove folder %1: %2").arg(firstArg, errnoToQString(errno)));
     }
     return removed;
 }
@@ -616,15 +600,11 @@ bool AppendFileOperation::performOperation()
 {
     // This operation takes two arguments. First argument is the name of the file into which a text has to be
     // appended. Second argument is the text to append.
-    const QStringList args = this->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 fName = args.first();
+    const QStringList args = this->arguments();
+    const QString fName = args.at(0);
     QFile file(fName);
     if (!file.open(QFile::Append)) {
         // first we rename the file, then we copy it to the real target and open the copy - the renamed original is then marked for deletion
@@ -653,7 +633,7 @@ bool AppendFileOperation::performOperation()
     }
 
     QTextStream ts(&file);
-    ts << args.last();
+    ts << args.at(1);
     file.close();
 
     return true;
@@ -727,14 +707,11 @@ bool PrependFileOperation::performOperation()
     // This operation takes two arguments. First argument is the name
     // of the file into which a text has to be appended. Second argument
     // is the text to append.
-    const QStringList args = this->arguments();
-    if (args.count() != 2) {
-        setError(InvalidArguments);
-        setErrorString(tr("Invalid arguments: %1 arguments given, 2 expected.").arg(args.count()));
+    if (!checkArgumentCount(2))
         return false;
-    }
 
-    const QString fName = args.first();
+    const QStringList args = this->arguments();
+    const QString fName = args.at(0);
     // Load the file first.
     QFile file(fName);
     if (!file.open(QFile::ReadOnly)) {
@@ -748,7 +725,7 @@ bool PrependFileOperation::performOperation()
     file.close();
 
     // Prepend text to the file text
-    fContents = args.last() + fContents;
+    fContents = args.at(1) + fContents;
 
     // Now re-open the file in write only mode.
     if (!file.open(QFile::WriteOnly)) {