Flame_Phoenix
Flame_Phoenix

Reputation: 17574

clean repeated code in C++ constructors

I have a header file in c++ that provides several constructors for user(it is a requirement):

#ifndef IMANDEL_H_
#define IMANDEL_H_

class IMandel{

public:
    IMandel();
    IMandel(int aWidth, int aLength);
    IMandel(int threads, int aWidth, int aLength);
    //other stuff!
private:
    int max_iterations, thread_count, height, width;
    int* buffer; 
};

#endif

Therefore in my corresponding cpp file I have these constructors implemented respectively:

//default constructor
IMandel::IMandel(){
    height = 10000;
    width = 10000;

    //this code segements gets repeated in every constructor! Messy!
    max_iterations = 255;
    thread_count = 1;
    buffer = new int[width*height];
}

IMandel::IMandel(int aWidth, int aLength){
    width = aWidth;
    height = aLength;

     //this code segements gets repeated in every constructor! Messy!
    max_iterations = 255;
    thread_count = 1;
    buffer = new int[width*height];
}

IMandel::IMandel(int threads, int aWidth, int aLength){
    thread_count = threads;
    width = aWidth;
    height = aLength;

     //this code segements gets repeated in every constructor! Messy!
    max_iterations = 255;
    buffer = new int[width*height];
}

As you can see, my constructors are not healthy, they have repeated chunks of code everywhere, and that is terrible!

In java , I found a solution to this problem by using constructors call each other. Basically I re-use the constructors like the following (Java example):

public myClass(){
    this(1, 10000, 10000);
}

public myClass(int aWidth, int aLength){
   this(1, aWidth, aLentgh);
}

public myClass(int threads, int aWidth, int aLength){
   thread_count = threads;
   width = aWidth;
   height = aLength;
   max_iterations = 255;
   buffer = new int[width*height];
}

As you can see int this Java example, there is no repeated code amongst the various constructors. Questions:

  1. Is there a way to achieve this same effect in C++?
  2. If so how? Can you provide a sample?
  3. If not, what solution do you recommend to fix the problem?

Upvotes: 0

Views: 799

Answers (3)

Cheers and hth. - Alf
Cheers and hth. - Alf

Reputation: 145269

What solution to adopt depends on the concrete case.

Here's your current class definition:

class IMandel
{
public:
    IMandel();
    IMandel(int aWidth, int aLength);
    IMandel(int threads, int aWidth, int aLength);
    //other stuff!
private:
    int max_iterations, thread_count, height, width;
    int* buffer; 
};

Here's how I would define it:

class IMandel
{
public:
    IMandel( int aWidth = 10000, int aLength = 10000, int threads = 1 );
    //other stuff!
private:
    int max_iterations, thread_count, height, width;
    std::vector<int> buffer;
};

Note that this still gives you the apparent constructors that you originally had, except for argument order.

There's no need to make things more complicated than necessary, that is, KEEP IT SIMPLE. Other C++03 means include, however, a common init function (when that is applicable, not here), and a common artifical base class. Other C++11 means include constructor forwarding.


In passing, note that your original code violated the "rule of three", i.e. it needed to either disallow or explicitly support copying, but failed to do that. The simpliciation presented above does not suffer from that problem. Can you see why?

Upvotes: 3

Drew Dormann
Drew Dormann

Reputation: 63775

The practical solution varies, depending on which version of C++ you are using.

In C++03, a common (but imperfect - see useful comments at the bottom) approach is to create an init() function that all the constructors call. All three of your constructors could be a single line calling a function like this:

void IMandel::init(int threads, int aWidth, int aLength){
    thread_count = threads;
    width = aWidth;
    height = aLength;

     //this code segements gets repeated in every constructor! Messy!
    max_iterations = 255;
    buffer = new int[width*height];
}

//default constructor
IMandel::IMandel(){
    init( 1, 10000, 10000 );
}

IMandel::IMandel(int aWidth, int aLength){
    init( 1, aWidth, aLength );
}

IMandel::IMandel(int threads, int aWidth, int aLength){
    init( threads, aWidth, aLength );
}

In C++11, constructors are allowed to call other constructors, as @chris commented. You could change your constructors in this manner:

//default constructor
IMandel::IMandel()
: IMandel( 1, 10000, 10000 )
{
}

IMandel::IMandel(int aWidth, int aLength)
: IMandel( 1, aWidth, aLength )
{
}

IMandel::IMandel(int threads, int aWidth, int aLength){
    thread_count = threads;
    width = aWidth;
    height = aLength;

     //this code segements gets repeated in every constructor! Messy!
    max_iterations = 255;
    buffer = new int[width*height];
}

Upvotes: 7

Praetorian
Praetorian

Reputation: 109119

If your compiler supports C++11's delegating constructors that can reduce having duplicate code in constructors. In your case though, it seems you just need to provide default values for your constructor arguments.

#ifndef IMANDEL_H_
#define IMANDEL_H_

class IMandel{

public:
    IMandel(int aWidth = 10000, int aLength = 10000, int threads = 1);
private:
    int max_iterations, thread_count, height, width;
    int* buffer; 
};

#endif

IMandel::IMandel(int aWidth, int aLength, int threads)
: max_iterations(255)
, thread_count(threads)
, height(aLength)
, width(aWidth)
, buffer(new int[width*height])
{
}

The above constructor does everything the 3 you've shown do. Also, consider using a vector<int> for buffer instead of managing the memory yourself.

Upvotes: 2

Related Questions