From d8bf6caebffcf24d1aec6a9c15653371549aaee3 Mon Sep 17 00:00:00 2001
From: Julien Michel <julien.michel@cnes.fr>
Date: Wed, 20 Jun 2018 17:32:20 +0200
Subject: [PATCH] ENH: Adressing comments from @poughov, with a bit of C++11
 seasoning

---
 .../include/otbWrapperCommandLineLauncher.h   |  9 +++
 .../src/otbWrapperCommandLineLauncher.cxx     | 67 ++++++++++++-------
 2 files changed, 52 insertions(+), 24 deletions(-)

diff --git a/Modules/Wrappers/CommandLine/include/otbWrapperCommandLineLauncher.h b/Modules/Wrappers/CommandLine/include/otbWrapperCommandLineLauncher.h
index 120c4ea4f6..5227dc1804 100644
--- a/Modules/Wrappers/CommandLine/include/otbWrapperCommandLineLauncher.h
+++ b/Modules/Wrappers/CommandLine/include/otbWrapperCommandLineLauncher.h
@@ -168,7 +168,16 @@ protected:
   unsigned int GetMaxKeySize() const;
 
 private:
+  /** \return false if paramKey is a missing mandatory parameter */
+  bool CheckMissingMandatoryParameter(const std::string & paramKey) const;
 
+  /** Prints a warning to std::cerr if paramKey is an unused parameter */
+  void CheckUnusedParameter(const std::string & paramKey) const;
+
+  /** \return false if paramKey is an OutputFilename parameter
+  pointing to a path that does not exist */
+  bool CheckOutputPathsValidity(const std::string & paramKey) const;
+  
   CommandLineLauncher(const CommandLineLauncher &) = delete;
   void operator =(const CommandLineLauncher&) = delete;
 
diff --git a/Modules/Wrappers/CommandLine/src/otbWrapperCommandLineLauncher.cxx b/Modules/Wrappers/CommandLine/src/otbWrapperCommandLineLauncher.cxx
index 2db623b3e3..48033a9949 100644
--- a/Modules/Wrappers/CommandLine/src/otbWrapperCommandLineLauncher.cxx
+++ b/Modules/Wrappers/CommandLine/src/otbWrapperCommandLineLauncher.cxx
@@ -511,18 +511,54 @@ CommandLineLauncher::ParamResultType CommandLineLauncher::LoadParameters()
       }
     }
 
-  // SECOND PASS : check mandatory parameters
-  for (unsigned int i = 0; i < appKeyList.size(); i++)
+  // SECOND PASS : checks
+  for (const auto & paramKey : appKeyList)
     {
-    const std::string paramKey(appKeyList[i]);
-    ParameterType type = m_Application->GetParameterType(paramKey);
+    // Check for missing mandatory parameters
+    if(!CheckMissingMandatoryParameter(paramKey))
+      return MISSINGMANDATORYPARAMETER;
+
+    // Check and warn unused parameters
+    CheckUnusedParameter(paramKey);
+
+    // Check output paths are valid
+    if(!CheckOutputPathsValidity(paramKey))
+      return WRONGPARAMETERVALUE;
+    }
+
+  return OKPARAM;
+}
+
+bool CommandLineLauncher::CheckOutputPathsValidity(const std::string & paramKey) const
+{
+  ParameterType type = m_Application->GetParameterType(paramKey);
+  if (m_Application->HasValue(paramKey) &&
+      type == ParameterType_OutputFilename)
+    {
+    std::string filename = m_Application->GetParameterString(paramKey);
+    itksys::String path = itksys::SystemTools::GetFilenamePath(filename);
+    if (path!="" && !itksys::SystemTools::FileIsDirectory(path.c_str()))
+      {
+      std::cerr <<"ERROR: Directory doesn't exist : "<< path.c_str() << std::endl;
+      return false;
+        }
+      }
+  return true;
+}
+
+bool CommandLineLauncher::CheckMissingMandatoryParameter(const std::string & paramKey) const
+{
     if (m_Application->IsParameterMissing(paramKey))
       {
       std::cerr << "ERROR: Missing mandatory parameter -" << paramKey << "." << std::endl;
-      return MISSINGMANDATORYPARAMETER;
+      return true;
       }
+    return false;
+}
 
-    // Check for ignored parameters
+void CommandLineLauncher::CheckUnusedParameter(const std::string & paramKey) const
+{
+  // Check for ignored parameters
     if(m_Application->HasUserValue(paramKey))
       {
 
@@ -551,30 +587,13 @@ CommandLineLauncher::ParamResultType CommandLineLauncher::LoadParameters()
           // Check that this choice is active
           if(paramKey.find(value) == std::string::npos)
             {
-            const std::string & missingMode = paramKey.substr(start,end-start);
-            std::cerr<<"WARNING: Parameter -"<<paramKey<<" will be ignored because -"<<key<<" is "<<value<<". Consider adding -"<<key<<" "<<missingMode<<" to application parameters."<<std::endl;
+            std::cerr<<"WARNING: Parameter -"<<paramKey<<" will be ignored because -"<<key<<" is "<<value<<"."<<std::endl;
             
             break;
             }
           }
         }
       }
-
-    // Check output paths validity
-    if (m_Application->HasValue(paramKey) &&
-        type == ParameterType_OutputFilename)
-      {
-      std::string filename = m_Application->GetParameterString(paramKey);
-      itksys::String path = itksys::SystemTools::GetFilenamePath(filename);
-      if (path!="" && !itksys::SystemTools::FileIsDirectory(path.c_str()))
-        {
-        std::cerr <<"ERROR: Directory doesn't exist : "<< path.c_str() << std::endl;
-        return WRONGPARAMETERVALUE;
-        }
-      }
-    }
-
-  return OKPARAM;
 }
 
 void CommandLineLauncher::LinkWatchers(itk::Object * itkNotUsed(caller), const itk::EventObject & event)
-- 
GitLab