How to Initialize Member Variables

Question

Probably an easy question for someone out there, but what am I doing wrong in the below example? I'm trying to build a global class which contains instantiations of other classes within... I think where I'm going wrong boils down to the below example. Getting a seg fault, as if *b is never created. Thanks in advance!!

#include <iostream>

using namespace std;

class A;
class B;

class B
 {
   public:
   B()
   {
       b = 99;
   }
   ~B();

  int Getb() {return b; }
  void Setb (int x) { b = x; }

  private:
  int b;

};

class A
{
    public:
   A()
    {
        B *b = new B;
    }
    ~A();

    B * b;

    void Printout()
    {
        cout<<b->Getb()<<endl;
    }
    private:

};

int main()
{
A *a = new A;
a->Printout();
cin.get();
}

Answer 1

In the cosntructor A::A() you don't initilize the A::b member, but a local variable instead. Try doing:

A() {
     b = new B;
}

or better:

A():b(new B) {}

And even better, don't use the raw pointer at all.

Answer 2

A() {
    B *b = new B;
}

B * b;

In the constructor you're declaring a new local variable that gets assigned the address of the freshly allocated B, and then forgotten!

The instance field b is never assigned to because it is shadowed by the local variable of the same name in the constructor.

You probably mean to do

A() {
    b = new B;
}

Answer 3

Great tips guys, even though in retrospect I obfuscated my issue with naming the int variable b (should've been anything but b!!). That said, you guys "pointed" me in the direction to initialization lists, destructors, and ultimately to the topic of composition. Many thanks How to implement class composition in C++?

Answer 4

Although quite a few people have pointed out one way of fixing the problem you're seeing, none seems (to me, anyway) to be giving advice about how to really make the code better.

Your definition of B is what's called a quasi-class. To make a long story short, your B can be simplified a lot without losing anything:

struct B { 
    int b;
    B() : b(99) {}
};

Everything else you've done (get/set, destructor) are accomplishing absolutely nothing. Your A class not only accomplishes just about as little, but does it even more poorly. Others have already pointed out the problem with A's constructor defining a local B object, and then leaking that object. None (that I've seen yet, anyway) has pointed out that even when you fix that, your definition of A will leak the B object anyway, because even though it creates a B object as part of creating an A object, it does not destroy the B object when the A object that contains it is destroyed.

I don't see any reason for your A class to dynamically allocate the B object at all (or, when you get down to it, to even exist). I'd define A more like this:

class A { 
     B b;
public:
     void print() { std::cout << b.b << "\n";
};

It would be better, however, if a B object knew how to insert itself into a stream -- and if it used the normal syntax for that as well:

std::ostream &operator<<(std::ostream &os, B const &b) { 
    return os << b.b;
}

With this in place, your A class adds nothing at all, so your entire program becomes something like this:

struct B { 
    int b;
    B() : b(99) {}
};

std::ostream &operator<<(std::ostream &os, B const &b) { 
    return os << b.b;
}

int main() {
    std::cout << B() << "\n";
    return 0;
}

Answer 5

B *b = new B;

Creates a local variable named b which shadows the class member b. You need to initialize the class member, and you should do it in an initialization list.

A() : b(new B) {}

Your next step is to fix the memory leak caused by never calling delete on the pointers you dynamically allocate, but since this is a learning exercise it's probably not terribly important (yet).

Answer 6

A()
{
    B *b = new B;
}

should be

A()
{
    b = new B;
}

In your version there a variable called b in the A constructor. This variable hides the A class member also called b (which was obviously the one you wanted to use).