13

I have a (for me) complex object with about 20 data member, many of which are pointer to other classes. So for the constructor, I have a big long, complex initialization list. The class also has a dozen different constructors, reflecting the various ways the class can be created. Most of these initialized items are unchanged between each of these different constructors.

My concern here is that I now have a large chuck of copied (or mostly copied) code which, if I need to add a new member to the class, may not make it into each of the constructor initialization lists.

class Object 
{
    Object();
    Object(const string &Name);
    Object (const string &Name, const string &path);
    Object (const string &Name, const bool loadMetadata);
    Object (const string &Name, const string &path, const bool loadMetadata);
} 

Object::Object() :
    name(),
    parent_index (0),
    rowData (new MemoryRow()),
    objectFile (),
    rows (new MemoryColumn (object_constants::RowName, OBJECTID, object_constants::ROWS_OID)),
    cols (new MemoryColumn (object_constants::ColName, OBJECTID, object_constants::COLS_OID)),
    objectName (new MemoryColumn(object_constants::ObjName, STRING, object_constants::short_name_len, object_constants::OBJECTNAME_OID)),
    parent     (new MemoryColumn(object_constants::ParentName, STRING, object_constants::long_name_len, object_constants::PARENT_OID)),
    parentIndex (new MemoryColumn(object_constants::ParentIndex, OBJECTID, object_constants::PARENTINDEX_OID)),
    childCount (new MemoryColumn (object_constants::ChildCount, INTEGER, object_constants::CHILD_COUNT_OID)),
    childList (new MemoryColumn (object_constants::ChildList, STRING, object_constants::long_name_len, object_constants::CHILD_OID)),
    columnNames (new MemoryColumn (object_constants::ColumnNames, STRING, object_constats::short_name_len, object_constants::COLUMN_NAME)),
    columnTypes (new MemoryColumn (object_constants::ColumnTypes, INTEGER, object_constants::COLUMN_TYPE)),
    columnSizes (new MemoryColumn (object_constants::ColumnSizes, INTEGER, object_constants::COLUMN_SIZE))
{}

Then repeat as above for the other constructors. Is there any smart way of using the default constructor for this, then modifying the results for the other constructors?

Thomas Jones-Low
  • 7,001
  • 2
  • 32
  • 36

12 Answers12

15

How about refactor the common fields into a base class. The default constructor for the base class would handle initialization for the plethora of default fields. Would look something like this:

class BaseClass {
    public:
    BaseClass();
};

class Object : public BaseClass
{
    Object();
    Object(const string &Name);
    Object (const string &Name, const string &path);
    Object (const string &Name, const bool loadMetadata);
    Object (const string &Name, const string &path, const bool loadMetadata);
};

BaseClass::BaseClass() :
    parent_index (0),
    rowData (new MemoryRow()),
    objectFile (),
    rows (new MemoryColumn (object_constants::RowName, OBJECTID, object_constants::ROWS_OID)),
    cols (new MemoryColumn (object_constants::ColName, OBJECTID, object_constants::COLS_OID)),
    objectName (new MemoryColumn(object_constants::ObjName, STRING, object_constants::short_name_len, object_constants::OBJECTNAME_OID)),
    parent     (new MemoryColumn(object_constants::ParentName, STRING, object_constants::long_name_len, object_constants::PARENT_OID)),
    parentIndex (new MemoryColumn(object_constants::ParentIndex, OBJECTID, object_constants::PARENTINDEX_OID)),
    childCount (new MemoryColumn (object_constants::ChildCount, INTEGER, object_constants::CHILD_COUNT_OID)),
    childList (new MemoryColumn (object_constants::ChildList, STRING, object_constants::long_name_len, object_constants::CHILD_OID)),
    columnNames (new MemoryColumn (object_constants::ColumnNames, STRING, object_constats::short_name_len, object_constants::COLUMN_NAME)),
    columnTypes (new MemoryColumn (object_constants::ColumnTypes, INTEGER, object_constants::COLUMN_TYPE)),
    columnSizes (new MemoryColumn (object_constants::ColumnSizes, INTEGER, object_constants::COLUMN_SIZE))
{}

Your Object constructors should look a little more manageable, now:

Object::Object() : BaseClass() {}
Object::Object (const string &Name): BaseClass(), name(Name) {}
Object::Object (const string &Name, const string &path): BaseClass(), name(Name), path_(path){}
Object::Object (const string &Name, const bool loadMetadata): BaseClass(), name(Name){}
Object::Object (const string &Name, const string &path, const bool loadMetadata): BaseClass(), path_(path) {}

Similar in nature to Iraimbilanja's answer, but avoids adding an inner-class for accessing data, which might impact a lot of existing code. If you've already got a class hierarchy, though, it may be difficult to factor it into a base class.

veefu
  • 2,820
  • 1
  • 19
  • 29
12

Now a couple of years later we have C++ 11. If you can use it in your project you have two options:

When the common initialization values are only known at runtime you can use delegating constructors, which means one constructor calls another.

 // function that gives us the init value at runtime.
 int getInitValue();

 class Foo
 {
     const int constant;
     int userSet;

 public:
     // initialize long member list with runtime values
     Foo() 
       : constant(getInitValue())
       , userSet(getInitValue())
     {}

     // other constructors with arguments
     Foo( int userSetArg) 
       : Foo()
       , userSet(userSetArg) 
     {
     }
 };

or you can initialize the members directly in the class definition if their values are known at compile time.

class Foo
{
    const int constant = 0;
    int userSet = 0;

public:
    Foo( int userSetArg) : userSet(userSetArg){}
}
Knitschi
  • 2,822
  • 3
  • 32
  • 51
5

Yes, it's possible.
For simplicity I'll pretend that the original code is:

class Foo {
public:
    Foo() : a(0), b(1), x() { }
    Foo(int x) : a(0), b(1), x(x) { }

    int get_a() const { return a; }
    int get_b() const { return b; }
    int get_x() const { return x; }
private:
    int a, b, x;
};

The refactored code, then, is:

class Foo {
public:
    Foo() : x() { }
    Foo(int x) : x(x) { }

    int get_a() const { return common.a; }
    int get_b() const { return common.b; }
    int get_x() const { return x; }
private:
    struct Common {
        Common() : a(0), b(1) { }
        int a, b;
    } common;
    int x;
};
4

Boost::Parameter makes it easy to implement Named Parameter Idiom. Check out this thread on SO. This may not be exactly what you need, but provides for some flexibility when you want to forward calls to the default ctor.

Community
  • 1
  • 1
dirkgently
  • 108,024
  • 16
  • 131
  • 187
4

Not to do with constructors, but why do you think you have to create all those sub-objects dynamically with new? This is not a good idea - you should avoid dynamic creation wherever possible. Don't make all those members pointers - make them actual objects.

  • 1
    That works some of the time. What if the class then becomes very large, and mostly unused? What if it's necessary to copy class objects fairly frequently? What if different members wind up sharing subobjects? – David Thornley Apr 17 '09 at 20:12
2

You can share their common code in a private init() member function.

Example:

class Object
{
 public:
   Object(const string &Name);
   Object(const string &Name, const string &path);
   ...
 private:
   void init();
 };

 Object::Object(const string &Name)
 {
   init();
   ...
 }

 Object::Object(const string &Name, const string &path)
 {
   init();
   ...
 }

 void Object::init()
 {
//intialization stuff
   ...
 } 
Naveen
  • 74,600
  • 47
  • 176
  • 233
aJ.
  • 34,624
  • 22
  • 86
  • 128
  • 3
    This means you are not initializing, you're post-initializing. This might be acceptable, but is usually slower, and, when you have members without a default ctor, impossible. –  Apr 17 '09 at 19:59
  • 1
    It will not work for references either. I would have loved to see C++ have a way to chain constructors to avoid this type of problem. – Martin York Apr 17 '09 at 20:08
  • I started to use this method, but all the c++ references I've found (books and web) all insist that using the initializer list was the way to go. – Thomas Jones-Low Apr 18 '09 at 11:19
  • 1
    Yes, initializer list should be preferred. But in this case I had to prefer init() to avoid the code duplication. Additional reference : http://www.parashift.com/c++-faq-lite/ctors.html#faq-10.3 – aJ. Apr 18 '09 at 12:04
1

First of all, you'll have memory leaks if you don't implement deletion of the allocated objects in the destructor. So you should define your destructor and delete the objects there.

If you really need to dynamically allocate the members (I don't recommended if this class owns the data member object), you can have a private method doing all the initialization and you can call that method from your constructors.

Cătălin Pitiș
  • 14,123
  • 2
  • 39
  • 62
  • all the values of new MemoryColum() are going into smart pointers. This will automatically delete them when Object is destroyed. In theory. – Thomas Jones-Low Apr 17 '09 at 22:06
1
Object (const string &Name = "", const string &path = "", const bool loadMetadata = false);

This won't solve all of your problems (in particular, there's no way to represent the constructor with Name and loadMetaData), but it will at least collapse some of the constructors into one.

David Thornley
  • 56,304
  • 9
  • 91
  • 158
1

I'll preface this by saying that I (obviously) don't know the details of your system, or the constraints that led to your design decisions.

That being said, a good rule of thumb is that when a class starts getting unweildy - around the time when you start asking questions about how to handle it :) - it may be time to refactor that class into a couple of smaller subclasses.

Remember that a class is supposed to do one thing very well. If you start to have large classes that try to do too many things, you're drifting away from good OO design.

e.James
  • 116,942
  • 41
  • 177
  • 214
1

I would use different factory methods (static methods) that would return back a smart ptr to your class. The factory method names would also help document WHY you need all the different parameters.

chrish
  • 2,352
  • 1
  • 17
  • 32
  • +1, I think that's a good idea, but I don't see how it gets around the problems mentioned by Iraimbilanja -- namely, how to initialise const members and references. You still need a variety of constructors if you want to offer defaults on these. – j_random_hacker Apr 18 '09 at 15:39
0

Do you really need 5 different constructors?

Very often if you need convert constructors you don't also want a default constructor.

Your other constructors all take just different combination of the same thing. It might be a better idea to have just one constructor that takes all of the parameters, with an option for each parameter that indicates the desire to invoke some kind of default for that parameter.

For example:

class Object
{
  Object (const string *Name, // can be NULL
    const string *path, // can be NULL 
    const bool loadMetadata);

};
John Dibling
  • 99,718
  • 31
  • 186
  • 324
  • Taking NULL pointers-to-string would be very unexpected, but as a last resort it's workable, yes. –  Apr 17 '09 at 20:08
0

Just put the initializer list inside a MACRO and be done with it.

BigSandwich
  • 2,768
  • 2
  • 22
  • 26