Refactor CalypOption

.github/workflows/build.yml

@@ -48,7 +48,6 @@ jobs:
       with:
         repository: 'pixlra/calyp-testdata'
         path: ${{github.workspace}}/tests_data
-        lfs: true
 
     - name: Configure CMake
       # Configure CMake in a 'build' subdirectory. `CMAKE_BUILD_TYPE` is only required if you are using a single-configuration generator such as make.

app/dialog/ConfigureFormatDialog.h

@@ -38,7 +38,7 @@ class QSpinBox;
 class QString;
 class QWidget;
 
-typedef QVector<CalypStandardResolution> CalypStandardResolutionVector;
+using CalypStandardResolutionVector = QVector<CalypStandardResolution>;
 QDataStream& operator<<( QDataStream& out, const CalypStandardResolutionVector& array );
 QDataStream& operator>>( QDataStream& in, CalypStandardResolutionVector& array );
 

app/dialog/ModulesHandleOptDialog.cpp

@@ -36,18 +36,17 @@
 
 class QLineEdit;
 
-class OpionConfiguration : public QWidget
+class OptionConfiguration : public QWidget
 {
 public:
-  OpionConfiguration( OptionBase* option )
+  OptionConfiguration( CalypOptions::OptionBase* option )
+      : m_cName{ option->opt_string }
   {
-    m_pcChecked = NULL;
-    m_cName = option->opt_string;
-    m_pcDescription = new QLabel;
+    auto* pcDescription = new QLabel;
     QString desc = QString::fromStdString( option->opt_desc );
-    m_pcDescription->setText( desc );
+    pcDescription->setText( desc );
     QHBoxLayout* layout = new QHBoxLayout;
-    layout->addWidget( m_pcDescription, 0, Qt::AlignLeft );
+    layout->addWidget( pcDescription, 0, Qt::AlignLeft );
     if( !option->isBinary() )
     {
       m_pcValue = new QLineEdit;
@@ -64,7 +63,7 @@ public:
     layout->setContentsMargins( 1, 1, 1, 1 );
     setLayout( layout );
   }
-  const QString getValue() const
+  auto getValue() const -> QString
   {
     if( m_pcChecked )
     {
@@ -72,12 +71,11 @@ public:
     }
     return m_pcValue->text();
   }
-  const std::string& getName() { return m_cName; }
+  auto getName() const -> std::string { return m_cName; }
 
 private:
-  QLineEdit* m_pcValue;
-  QCheckBox* m_pcChecked;
-  QLabel* m_pcDescription;
+  QLineEdit* m_pcValue{ nullptr };
+  QCheckBox* m_pcChecked{ nullptr };
   std::string m_cName;
 };
 
@@ -87,13 +85,12 @@ ModulesHandleOptDialog::ModulesHandleOptDialog( QWidget* parent, CalypAppModuleI
   resize( 400, 10 );
   setWindowTitle( "Select module parameters" );
 
-  const CalypOptions::OptionsList& moduleOptions = m_pcCurrModuleIf->m_pcModule->m_cModuleOptions.getOptionList();
+  auto& moduleOptions = m_pcCurrModuleIf->m_pcModule->m_cModuleOptions.getOptionList();
 
   QVBoxLayout* optionsLayout = new QVBoxLayout;
-  OpionConfiguration* pcOption;
-  for( CalypOptions::OptionsList::const_iterator it = moduleOptions.begin(); it != moduleOptions.end(); ++it )
+  for( auto& option : moduleOptions )
   {
-    pcOption = new OpionConfiguration( ( *it )->opt );
+    auto pcOption = new OptionConfiguration( option->base_opt.get() );
     m_apcOptionList.append( pcOption );
     optionsLayout->addWidget( pcOption );
   }
@@ -125,13 +122,13 @@ int ModulesHandleOptDialog::runConfiguration()
   std::string optionString;
   QString valueString( "" );
 
-  for( int i = 0; i < m_apcOptionList.size(); i++ )
+  for( const auto& option : m_apcOptionList )
   {
-    valueString = m_apcOptionList.at( i )->getValue();
+    valueString = option->getValue();
     if( !valueString.isEmpty() )
     {
       optionString.append( "--" );
-      optionString.append( m_apcOptionList.at( i )->getName() );
+      optionString.append( option->getName() );
       if( !( valueString == "true" ) )
       {
         optionString.append( "=" );

app/dialog/ModulesHandleOptDialog.h

@@ -32,7 +32,7 @@
 #include "CommonDefs.h"
 #include "config.h"
 
-class OpionConfiguration;
+class OptionConfiguration;
 
 /**
  *
@@ -47,7 +47,7 @@ public:
 
 private:
   CalypAppModuleIf* m_pcCurrModuleIf;
-  QVector<OpionConfiguration*> m_apcOptionList;
+  QVector<OptionConfiguration*> m_apcOptionList;
 };
 
 #endif  // __MODULEHANDLEOPTDIALOG_H__

lib/CalypOptions.cpp

@@ -40,6 +40,7 @@
 
 #include <cstring>
 #include <iostream>
+#include <memory>
 #include <sstream>
 
 #include "config.h"
@@ -52,32 +53,33 @@ struct ParseFailure : public std::exception
   std::string arg;
   std::string val;
   ParseFailure( std::string arg0, std::string val0 ) throw()
-      : arg( arg0 ), val( val0 ) {}
+      : arg{ std::move( arg0 ) }, val{ std::move( val0 ) } {}
+  ParseFailure( ParseFailure&& other ) noexcept = delete;
+  ParseFailure& operator=( ParseFailure&& other ) noexcept = delete;
+  ParseFailure( const ParseFailure& other ) = delete;
+  ParseFailure& operator=( const ParseFailure& other ) = delete;
   ~ParseFailure() throw() {}
-  const char* what() const throw() { return "Option Parse Failure"; }
+  const char* what() const throw() override { return "Option Parse Failure"; }
 };
 
 /** Type specific option storage */
-class boolOption : public OptionBase
+class boolOption : public CalypOptions::OptionBase
 {
 public:
   boolOption( const std::string& name, const std::string& desc )
-      : OptionBase( name, desc, "(1-0)" ) { is_binary = true; }
-  void parse( const std::string& arg ) { arg_count++; }
+      : CalypOptions::OptionBase( name, desc, "(1-0)" ) { is_binary = true; }
+  void parse( const std::string& arg ) override { arg_count++; }
 };
 
 /** Type specific option storage */
 template <typename T>
-class StandardOption : public OptionBase
+class StandardOption : public CalypOptions::OptionBase
 {
 public:
   StandardOption( const std::string& name, T& storage, const std::string& desc, const std::string& def )
-      : OptionBase( name, desc, def ), opt_storage( storage )
-  {
-    is_binary = false;
-  }
+      : CalypOptions::OptionBase( name, desc, def ), opt_storage( storage ) {}
 
-  void parse( const std::string& arg );
+  void parse( const std::string& arg ) override;
   T& opt_storage;
   std::vector<T> opt_storage_array;
 };
@@ -105,7 +107,7 @@ inline void StandardOption<T>::parse( const std::string& arg )
 template <>
 inline void StandardOption<std::vector<unsigned int>>::parse( const std::string& arg )
 {
-  unsigned int aux_opt_storage;
+  unsigned int aux_opt_storage{ 0 };
   std::istringstream arg_ss( arg, std::istringstream::in );
   arg_ss.exceptions( std::ios::failbit );
   try
@@ -123,7 +125,7 @@ inline void StandardOption<std::vector<unsigned int>>::parse( const std::string&
 template <>
 inline void StandardOption<std::vector<int>>::parse( const std::string& arg )
 {
-  int aux_opt_storage;
+  int aux_opt_storage{ 0 };
   std::istringstream arg_ss( arg, std::istringstream::in );
   arg_ss.exceptions( std::ios::failbit );
   try
@@ -155,26 +157,26 @@ inline void StandardOption<std::vector<std::string>>::parse( const std::string&
 }
 
 /** Option class for argument handling using a user provided function */
-struct FunctionOption : public OptionBase
+struct FunctionOption : public CalypOptions::OptionBase
 {
-public:
-  typedef void( Func )( CalypOptions&, const std::string& );
+  using OptFunc = std::function<void( CalypOptions&, const std::string& )>;
 
-  FunctionOption( const std::string& name, CalypOptions& parent_, Func* func_, const std::string& desc, const std::string& def )
-      : OptionBase( name, desc, def ), parent( parent_ ), func( func_ )
+public:
+  FunctionOption( const std::string& name, CalypOptions& parent, OptFunc func, const std::string& desc, const std::string& def )
+      : CalypOptions::OptionBase( name, desc, def ), m_parent( parent ), m_func( func )
   {
     is_binary = false;
   }
 
-  void parse( const std::string& arg )
+  void parse( const std::string& arg ) override
   {
-    func( parent, arg );
+    m_func( m_parent, arg );
     arg_count++;
   }
 
 private:
-  CalypOptions& parent;
-  void ( *func )( CalypOptions&, const std::string& );
+  CalypOptions& m_parent;
+  OptFunc m_func;
 };
 
 /* Helper method to initiate adding options to Options */
@@ -187,7 +189,7 @@ private:
  */
 CalypOptions& CalypOptions::operator()( const std::string& name, const std::string& desc )
 {
-  addOption( new boolOption( name, desc ) );
+  addOption( std::make_unique<boolOption>( name, desc ) );
   return *this;
 }
 
@@ -199,14 +201,14 @@ CalypOptions& CalypOptions::operator()( const std::string& name, const std::stri
 template <typename T>
 CalypOptions& CalypOptions::operator()( const std::string& name, T& storage, const std::string& desc )
 {
-  addOption( new StandardOption<T>( name, storage, desc, std::string( "" ) ) );
+  addOption( std::make_unique<StandardOption<T>>( name, storage, desc, std::string( "" ) ) );
   return *this;
 }
 
 template <typename T>
 CalypOptions& CalypOptions::operator()( const std::string& name, T& storage, const std::string& desc, const std::string& defaults )
 {
-  addOption( new StandardOption<T>( name, storage, desc ) );
+  addOption( std::make_unique<StandardOption<T>>( name, storage, desc ) );
   return *this;
 }
 
@@ -223,53 +225,35 @@ template CalypOptions& CalypOptions::operator()( const std::string& name, std::v
 template CalypOptions& CalypOptions::operator()( const std::string& name, std::vector<std::string>& storage,
                                                  const std::string& desc );
 
-CalypOptions::Option::Option()
-    : opt( 0 ) {}
-
-CalypOptions::Option::~Option()
+CalypOptions::CalypOptions( std::string name )
+    : m_cOptionGroupName{ std::move( name ) }
 {
-  if( opt )
-    delete opt;
 }
 
-CalypOptions::CalypOptions( const std::string& name )
-{
-  m_cOptionGroupName = name;
-  m_bAllowUnkonw = true;
-}
-
-CalypOptions::~CalypOptions()
-{
-  for( CalypOptions::OptionsList::iterator it = opt_list.begin(); it != opt_list.end(); it++ )
-  {
-    delete *it;
-  }
-}
-
-OptionBase* CalypOptions::operator[]( const std::string& optName )
+CalypOptions::OptionBase* CalypOptions::operator[]( const std::string& optName )
 {
   return getOption( optName );
 }
 
-OptionBase* CalypOptions::getOption( const std::string& optName )
+CalypOptions::OptionBase* CalypOptions::getOption( const std::string& optName )
 {
-  CalypOptions::OptionMap::iterator opt_it;
+  std::map<std::string, std::list<Option*>>::iterator opt_it;
   opt_it = opt_short_map.find( optName );
   if( opt_it != opt_short_map.end() )
   {
-    OptionsList opt_list = ( *opt_it ).second;
-    for( CalypOptions::OptionsList::iterator it = opt_list.begin(); it != opt_list.end(); ++it )
+    auto opt_list = ( *opt_it ).second;
+    for( auto& option : opt_list )
     {
-      return ( *it )->opt;
+      return option->base_opt.get();
     }
   }
   opt_it = opt_long_map.find( optName );
   if( opt_it != opt_long_map.end() )
   {
-    OptionsList opt_list = ( *opt_it ).second;
-    for( CalypOptions::OptionsList::iterator it = opt_list.begin(); it != opt_list.end(); ++it )
+    auto opt_list = ( *opt_it ).second;
+    for( auto& option : opt_list )
     {
-      return ( *it )->opt;
+      return option->base_opt.get();
     }
   }
   return NULL;
@@ -281,26 +265,16 @@ bool CalypOptions::hasOpt( const std::string& optName )
   return opt ? opt->count() > 0 ? true : false : false;
 }
 
-void CalypOptions::addDefaultOptions()
-{
-  addOptions()                                                /**/
-      ( "help", "produce help message" )                      /**/
-      ( "version", "show version and exit" )                  /**/
-      ( "pel_fmts", "list pixel formats" )                    /**/
-      ( "quality_metrics", "list supported quality metrics" ) /**/
-      ( "module_list", "list supported modules" )             /**/
-      ( "module_list_full", "detailed list supported modules" );
-}
-
 CalypOptions& CalypOptions::addOptions()
 {
   return *this;
 }
 
-void CalypOptions::addOption( OptionBase* opt )
+void CalypOptions::addOption( std::unique_ptr<OptionBase> opt )
 {
-  Option* names = new Option();
-  names->opt = opt;
+  auto names = std::make_unique<Option>();
+  names->base_opt = std::move( opt );
+  opt = nullptr;
   string& opt_string = opt->opt_string;
 
   size_t opt_start = 0;
@@ -317,32 +291,33 @@ void CalypOptions::addOption( OptionBase* opt )
     if( force_short || opt_name.size() == 1 )
     {
       names->opt_short.push_back( opt_name );
-      opt_short_map[opt_name].push_back( names );
+      opt_short_map[opt_name].push_back( names.get() );
     }
     else
     {
       names->opt_long.push_back( opt_name );
-      opt_long_map[opt_name].push_back( names );
+      opt_long_map[opt_name].push_back( names.get() );
     }
     opt_start += opt_end + 1;
   }
-  opt_list.push_back( names );
+  opt_list.push_back( std::move( names ) );
 }
 
-static void setOptions( CalypOptions::OptionsList& opt_list, const string& value )
+static void setOptions( std::list<CalypOptions::Option*>& opt_list, const string& value )
 {
   /* multiple options may be registered for the same name:
-   *   allow each to parse value */
-  for( CalypOptions::OptionsList::iterator it = opt_list.begin(); it != opt_list.end(); ++it )
+   *   allow each to parse value
+   */
+  for( auto& option : opt_list )
   {
-    ( *it )->opt->parse( value );
+    option->base_opt->parse( value );
   }
 }
 
 bool CalypOptions::storePair( bool allow_long, bool allow_short, const string& name, const string& value )
 {
   bool found = false;
-  CalypOptions::OptionMap::iterator opt_it;
+  std::map<std::string, std::list<Option*>>::iterator opt_it;
   if( allow_long )
   {
     opt_it = opt_long_map.find( name );
@@ -384,7 +359,7 @@ bool CalypOptions::storePair( const string& name, const string& value )
 /**
  * returns number of extra arguments consumed
  */
-unsigned int CalypOptions::parseLONG( unsigned int argc, char* argv[] )
+unsigned int CalypOptions::parseLONG( unsigned int argc, char* argv[] )  // NOLINT
 {
   /* gnu style long options can take the forms:
    *  --option=arg
@@ -454,7 +429,7 @@ unsigned int CalypOptions::parseLONG( string arg )
   return extra_argc_consumed;
 }
 
-unsigned int CalypOptions::parseSHORT( unsigned int argc, char* argv[] )
+unsigned int CalypOptions::parseSHORT( unsigned int argc, char* argv[] )  // NOLINT
 {
   /* short options can take the forms:
    *  --option arg
@@ -477,7 +452,7 @@ unsigned int CalypOptions::parseSHORT( unsigned int argc, char* argv[] )
   return 1;
 }
 
-int CalypOptions::parse( unsigned int argc, char* argv[] )
+int CalypOptions::parse( unsigned int argc, char* argv[] )  // NOLINT
 {
   try
   {
@@ -499,6 +474,7 @@ int CalypOptions::parse( unsigned int argc, char* argv[] )
   return 0;
 }
 
+// NOLINTNEXTLINE(*-avoid-c-arrays)
 list<const char*> CalypOptions::scanArgv( unsigned int argc, char* argv[] )
 {
   /* a list for anything that didn't get handled as an option */
@@ -592,10 +568,10 @@ void CalypOptions::doHelp( ostream& out, unsigned int columns )
   const unsigned int pad_short = 3;
   /* first pass: work out the longest option name */
   unsigned int max_width = 0;
-  for( CalypOptions::OptionsList::iterator it = opt_list.begin(); it != opt_list.end(); it++ )
+  for( auto& opt : opt_list )
   {
     ostringstream line( ios_base::out );
-    doHelpOpt( line, **it, pad_short );
+    doHelpOpt( line, *opt, pad_short );
     max_width = max( max_width, (unsigned int)line.tellp() );
   }
 
@@ -607,13 +583,13 @@ void CalypOptions::doHelp( ostream& out, unsigned int columns )
    *  - if the option text is longer than opt_width, place the help
    *    text at opt_width on the next line.
    */
-  for( CalypOptions::OptionsList::iterator it = opt_list.begin(); it != opt_list.end(); it++ )
+  for( auto& opt : opt_list )
   {
     ostringstream line( ios_base::out );
     line << "  ";
-    doHelpOpt( line, **it, pad_short );
+    doHelpOpt( line, *opt, pad_short );
 
-    const string& opt_desc = ( *it )->opt->opt_desc;
+    const string& opt_desc = opt->base_opt->opt_desc;
     if( opt_desc.empty() )
     {
       /* no help text: output option, skip further processing */

lib/CalypOptions.h

@@ -46,30 +46,6 @@
 
 #include "CalypDefs.h"
 
-/**
- * \class    OptionBase
- * \ingroup  CalypLibGrp
- * \brief    Virtual base class for storing information relating to a
- * specific option This base class describes common elements.
- * Type specific information should be stored in a derived class.
- */
-class OptionBase
-{
-public:
-  OptionBase( const std::string& name, const std::string& desc, const std::string& def )
-      : arg_count( 0 ), opt_string( name ), opt_desc( desc ), opt_defaults( def ) {}
-  virtual ~OptionBase() {}
-  /* parse argument arg, to obtain a value for the option */
-  virtual void parse( const std::string& arg ) = 0;
-  int count() { return arg_count; }
-  bool isBinary() { return is_binary; }
-  int arg_count;
-  std::string opt_string;
-  std::string opt_desc;
-  std::string opt_defaults;
-  bool is_binary;
-};
-
 /**
  * \class    CalypOptions
  * \ingroup  CalypLibGrp
@@ -78,34 +54,56 @@ public:
 class CalypOptions
 {
 public:
+  /**
+   * Virtual base class for storing information relating to a
+   * specific option This base class describes common elements.
+   * Type specific information should be stored in a derived class.
+   */
+  class OptionBase
+  {
+  public:
+    OptionBase( std::string name, std::string desc, std::string def )
+        : opt_string{ std::move( name ) }, opt_desc{ std::move( desc ) }, opt_defaults{ std::move( def ) } {}
+    OptionBase( OptionBase&& other ) noexcept = delete;
+    OptionBase& operator=( OptionBase&& other ) noexcept = delete;
+    OptionBase( const OptionBase& other ) = delete;
+    OptionBase& operator=( const OptionBase& other ) = delete;
+    virtual ~OptionBase() = default;
+    /* parse argument arg, to obtain a value for the option */
+    virtual void parse( const std::string& arg ) = 0;
+    auto count() const -> std::size_t { return arg_count; }
+    auto isBinary() const -> bool { return is_binary; }
+    std::string opt_string;
+    std::string opt_desc;
+    std::string opt_defaults;
+
+  protected:
+    std::size_t arg_count{ 0 };
+    bool is_binary{ false };
+  };
   struct Option
   {
-    Option();
-    ~Option();
     std::list<std::string> opt_long;
     std::list<std::string> opt_short;
-    OptionBase* opt;
+    std::unique_ptr<OptionBase> base_opt{ nullptr };
   };
-  typedef std::list<Option*> OptionsList;
 
-  CalypOptions( const std::string& name = "" );
-  ~CalypOptions();
+  CalypOptions() = default;
+  CalypOptions( std::string name );
 
-  int parse( unsigned int argc, char* argv[] );
+  int parse( unsigned int argc, char* argv[] );  // NOLINT
   void parse( std::vector<std::string> args_array );
 
   std::list<const char*>& getUnhandledArgs() { return m_aUnhandledArgs; }
   void doHelp( std::ostream& out, unsigned columns = 80 );
 
-  OptionBase* operator[]( const std::string& optName );
-  OptionBase* getOption( const std::string& optName );
+  CalypOptions::OptionBase* operator[]( const std::string& optName );
+  CalypOptions::OptionBase* getOption( const std::string& optName );
 
   bool hasOpt( const std::string& optName );
 
-  OptionsList getOptionList() { return opt_list; }
-  void addDefaultOptions();
+  auto getOptionList() -> const std::list<std::unique_ptr<Option>>& { return opt_list; }
   CalypOptions& addOptions();
-  void addOption( OptionBase* opt );
 
   /**
    * Add option described by name to the parent Options list,
@@ -130,22 +128,24 @@ public:
   bool checkListingOpts();
 
 private:
-  typedef std::map<std::string, OptionsList> OptionMap;
-  OptionMap opt_long_map;
-  OptionMap opt_short_map;
+  void addOption( std::unique_ptr<OptionBase> opt );
 
-  OptionsList opt_list;
+private:
+  std::map<std::string, std::list<Option*>> opt_long_map;
+  std::map<std::string, std::list<Option*>> opt_short_map;
 
-  std::string m_cOptionGroupName;
-  bool m_bAllowUnkonw;
+  std::list<std::unique_ptr<Option>> opt_list;
+
+  std::string m_cOptionGroupName{ "" };
+  bool m_bAllowUnkonw{ true };
   std::list<const char*> m_aUnhandledArgs;
 
   bool storePair( bool allow_long, bool allow_short, const std::string& name, const std::string& value );
   bool storePair( const std::string& name, const std::string& value );
-  unsigned int parseLONG( unsigned int argc, char* argv[] );
+  unsigned int parseLONG( unsigned int argc, char* argv[] );  // NOLINT
   unsigned int parseLONG( std::string arg );
-  unsigned int parseSHORT( unsigned int argc, char* argv[] );
-  std::list<const char*> scanArgv( unsigned int argc, char* argv[] );
+  unsigned int parseSHORT( unsigned int argc, char* argv[] );          // NOLINT
+  std::list<const char*> scanArgv( unsigned int argc, char* argv[] );  // NOLINT
 };
 
 #endif  // __CALYPOPTIONS_H__

tools/CalypToolsCmdParser.cpp

@@ -40,7 +40,14 @@ CalypToolsCmdParser::CalypToolsCmdParser()
   m_bQuiet = false;
   m_iFrames = -1;
 
-  m_cOptions.addDefaultOptions();
+  m_cOptions.addOptions()                                     /**/
+      ( "help", "produce help message" )                      /**/
+      ( "version", "show version and exit" )                  /**/
+      ( "pel_fmts", "list pixel formats" )                    /**/
+      ( "quality_metrics", "list supported quality metrics" ) /**/
+      ( "module_list", "list supported modules" )             /**/
+      ( "module_list_full", "detailed list supported modules" );
+  ;
 }
 
 CalypToolsCmdParser::~CalypToolsCmdParser() {}