bcf
bcf

Reputation: 2134

Consruct Member Class using Data computed in Composing Class

I have a class Itch that is a member of the class Scratch. I want to do some computations in the Scratch constructor and pass the result of these computations to instantiate the Itch object. My best guess in doing this is below, but this returns garbage:

#include <iostream>

class Itch {
public:
  int N;
  Itch(int n) {N = n;}
};

class Scratch {
private:
  int N;
public:
  Itch it;
  Scratch(int n);
};

Scratch::Scratch(int n) : it(N)  // here is where I want to use new data
{
  // do some silly things
  int temp = 5;
  temp += n + 45;
  N = temp - 1;
}

int main() {
  int n = 1;
  Scratch sc(n);

  std::cout << sc.it.N << "\n";
}

Is there a standard way to do this?

Upvotes: 1

Views: 28

Answers (1)

Jason C
Jason C

Reputation: 40315

The things in the initializer list happen before the things in the constructor code. Therefore, you cannot affect anything in the initializer list with the code in the constructor. You have a few options.

A reasonable approach would be to have an Itch * member rather than an Itch, and initialize it when it's ready, e.g.:

class Scratch {
  ...
  Itch *it;
  ...
};

Scratch::Scratch(int n) : it(NULL)
{
  // do some silly things
  int temp = 5;
  temp += n + 45;
  N = temp - 1;
  it = new Itch(N); // <- now you have enough info to instantiate an Itch
}

And you'll have to remember to clean up in the destructor unless you use an auto_ptr:

Scratch::~Scratch () {
  delete it;
}

Another reasonable approach would be to pass n to the Itch constructor and have it do the calculations there instead of in Scratch, perhaps even allowing Itch to determine N, e.g.:

class Itch {
private:
   int N;
public:
   Itch (int n);
   int getN () const { return N; }
}

Itch::Itch (int n) {
  // do some silly things
  int temp = 5;
  temp += n + 45;
  N = temp - 1;
}

Scratch::Scratch (int n) : it(n) {
  // you can either remove Scratch::N altogether, or I suppose do:
  N = it.getN();
  // ... do whatever makes sense, try not to have redundant data.
  // (also ask yourself if Scratch even *needs* to know N, or at
  // least if it can just use it.getN() everywhere instead of
  // keeping its own copy.)
}

Another approach, which IMO is a bit odd but it's still possible in some situations, is to have e.g. a static function (member or not) that computes N from n, which you can use in the initializer list, e.g.:

static int doSillyThings (int n) {
  int temp = 5;
  temp += n + 45;
  return temp - 1;
}

Scratch::Scratch(int n) : N(doSillyThings(n)), it(N)
{
}

Choose whichever leads to the cleanest, most maintainable and easy-to-read code. Personally I'd prefer the first, Itch * option, since it makes logical sense and is very clear: You do the calculations necessary to initialize the Itch, then you initialize it.

You should think about your code a bit. If the Scratch's N is always equal to it.N, then do you really need both Ns?

There are other options too (including restructuring your code completely so you don't have to have an Itch member of Scratch, or so that you don't have to have it depend on extra calculations done on the Scratchs constructor parameters but that really depends on the situation), but hopefully that inspires you a little.


The reason your code returns garbage, by the way, is because N is garbage at the point you pass it to the Itch constructor. It's uninitialized until you initialize it, and at the point where it(N) is you haven't initialized N yet.

Upvotes: 2

Related Questions