simplest syntax for passing in an array of subclasses of a polymorphic base class into a constructor

94 Views Asked by At

Here's working code, that I'd like to simplify the calling of. Let's start with the call:

Config c2( {
           new ConfigPairInt(  "one",   1   ),
           new ConfigPairDouble( "two",   2.0 ),
           new ConfigPairString( "three", "3" )
} );

The first thing I'd like to get rid of is the new's. (If I do new like this I suppose it's a memory leak?) However when I make Config's constructor take vector<ConfigPair&> and remove these new's I get pages of errors that don't make sense to me.

Second thing I'd like to get rid of is the type class names. If I instead switch to curly-brace initializers, then the compiler does know that the braced-initializer list is ConfigPair* and so everything it sees should be an initializer for ConfigPair or one of its subclasses. But does the compiler actually search subclass constructors? Apparently not. (I do have an idea which is to make ConfigPair do the job of all its subclasses, and give it three different constructors. It'd no longer be polymorphic. And not being polymorphic, I could then also get rid of the new because the vector could be of this ConfigPair class instead of ConfigPair* which is needed because of the polymorphism. However the polymorphism seems to fit the problem perfectly so I'm loathe to abandon it.)

Third thing I'd like to get rid of is the braces around the vector initializer. I think I could do this with a parameter pack but I can't get the resulting list of subclass parameters to initialize the vector.

class ConfigPair {
public:
  ConfigPair( const char* pszName_in ) :
    pszName( newstring( pszName_in ) )
  {
  };

  virtual ~ConfigPair() {
    free( (char*) pszName );
  };

  const char* pszName;
};



class Config {
public:
 
  Config( std::vector<ConfigPair*> apcpair )
  {
      for( ConfigPair* pcpair: apcpair )
          // do something
  }
};



class ConfigPairInt : public ConfigPair {
public:
  ConfigPairInt( const char* pszName_in, int iValue_in ) :
    ConfigPair( pszName_in ),
    iValue( iValue_in )
  {
  };

  int         iValue;
};



class ConfigPairDouble : public ConfigPair {
public:
  ConfigPairDouble( const char* pszName_in, double dValue_in ) :
    ConfigPair( pszName_in ),
    dValue( dValue_in )
  {
  };
  double      dValue;
};

  

class ConfigPairString : public ConfigPair {
public:
  ConfigPairString( const char* pszName_in, const char* pszValue_in ) :
    ConfigPair( pszName_in ),
    pszValue( newstring( pszValue_in ) )
  {
  };

  virtual ~ConfigPairString() {
    free( (char*) pszValue );
  };

  const char* pszValue;
};
3

There are 3 best solutions below

0
On BEST ANSWER

AS Remy and Panta point out in other answers: You simply can't get rid of the derived type names if you want to use polymorphism, since the compiler needs to know which types to create. Since this is the most typing for the caller, to eliminate it is a priority. Thus polymorphism is ruled out.

Instead, what was a virtual base class for different kinds of pairs now becomes a "Swiss Army Knife" able to do the job of what were its subclasses. It has various constructors, and the added overhead of an enum to know which constructor was called.

public:
  ConfigPair( const char* pszName_in, int iValue_in ) {
    cfgpair.type    = TypeInt64;
    cfgpair.pszName = newstring( pszName_in );
    cfgpair.u.i     = iValue_in;
  }

  ConfigPair( const char* pszName_in, double dValue_in ) {
    cfgpair.type    = TypeDouble;
    cfgpair.pszName = newstring( pszName_in );
    cfgpair.u.d     = dValue_in;
  }

  ConfigPair( const char* pszName_in, const char* pszValue_in ) {
    cfgpair.type     = TypeSz;
    cfgpair.pszName  = newstring( pszName_in );
    cfgpair.pszValue = newstring( pszValue_in );
  }

Given that ConfigPair is now the only class being passed into the Config constructor means that compiler's awareness of that constructor's type requirement allows it to deduce the object type when called. Thus, the class name is dispensed with.

Further, since the array contents are now homogenous, we can put the ConfigPair's themselves into an array, instead of (as was initially necessary) only storing pointers to the ConfigPair subclass objects. Thus, the new (or the alternative and non-memory-leaking but even more verbose shared_pointer<ConfigPairXXX>() notation).

  Config( const char* pszName_in, // name only used for debugging messages
          std::vector<ConfigPair> acpair ) :
      Config( pszName_in )
  {
      for ( const ConfigPair cpair: acpair )
          cpair.Set( this );
  }

And for the caller:

Config c2( { { "one",   1   },
             { "two",   2.0 },
             { "three", "3" } } );

Finally, that leaves the outer curly brackets for the braced-initializer-list. I explored replacing the vector with a parameter pack but then I can't figure out how to make the compiler aware of the type any more, so the type again would be required. To proceed with this would be one step forward ten steps back in terms of simplicity for the caller, so I am sticking with the braced-initializer-list after all. (If there's another alternative to get rid of these outer braces while not needing class names, please let me know.)

8
On

Here's a raw draft of what I would do in your case (as far as described), explanations in comments:

using ValueType = std::variant<std::string,int,double>; // To get rid of lengthy
                                                        // type names, and 
                                                        // have them
                                                        // change- and  maintainable 
                                                        // at a single point in your 
                                                        // source code
class ConfigPair {
public:
  ConfigPair( const std::string& name_in, ValueType value ) : 
                              // ^^^^^^^ Ditch "Hungarian notation" for heaven's sake!
    name( name_in ) {}

  virtual ~ConfigPair() {}; // Destructor doesn't need to do anything
                            // std::string manages memory already

  std::string name; // Is compatible with const char* for initialization
  ValueType  value;
};

using PConfigPair = std::shared_ptr<ConfigPair>;
// or in case that Config should take full ownership of ConfigPair's
// using PConfigPair = std::shared_ptr<ConfigPair>;

class Config {
public:
  Config( std::vector<PConfigPair> apcpair )
  {
      for( PConfigPair pcpair: apcpair )
          // do something
  }
// Maybe store the vector here as class member, if Config should take full
// ownership of those ConfigPair's
// std::vector<PConfigPair> config_pairs;
//
// In any case you don't need to care about manual memory management
};

Config c2( { std::make_shared<ConfigPair>("one",1), // std::make_unique alternatively
             std::make_shared<ConfigPair>("two",2.0),
             std::make_shared<ConfigPair>("three","3") } );

Docs:

6
On

You can get rid of the use of new and vector easily enough, by using a variadic template for the Config constructor, eg:

class Config
{
public:
    template<typename... Ts>
    Config(const Ts&... args)
    {
        const ConfigPair* arr[] = {&args...};
        // use arr as needed...
        for( const ConfigPair *arg: arr ) {
            // do something with arg...
        }
    }
};

int main()
{
    Config c2(
        ConfigPairInt("one", 1),
        ConfigPairDouble("two", 2.0),
        ConfigPairString("three", "3")
    );
    // use c2 as needed...
    return 0;
}

Live Demo

You simply can't get rid of the derived type names if you want to use polymorphism, since the compiler needs to know which types to create. Using a brace-initializer by itself doesn't tell the compiler WHICH type to make:

Config c2(
    {"one", 1},    // error: WHICH type?
    {"two", 2.0},  // error: WHICH type?
    {"three", "3"} // error: WHICH type?
);

If you want this kind of syntax, then get rid of the polymorphism altogether, just have ConfigPair itself handle any value type, via std::variant, std::any, or equivalent.