0

I am using a routine to initialize aggregate members of a class.

class MyClass
{
public:
    void Init()
    {
        double  array2D[10][10] = {0.0};
        bool    logicalarray[2] = {false};
    }

private:
    double  array2D[10][10];  
    bool    logicalarray[2]; 
};

int main()
{
    MyClass* myClassObject = new  MyClass();
    myClassObject->Init();

    delete myClassObject;
    return 0;
}

Is there anything wrong with this? It compiles, builds and runs fine. While debugging, I do see that each array element is initialized exactly the way I have defined in Init(). However, code cleanup tool reports that in Init(), named variables are declared but not referenced.

PS: No C++11

ontherocks
  • 1,747
  • 5
  • 26
  • 43

1 Answers1

1

The code cleanup tool is right. This code doesn't do what you think it does:

void Init()
    {
        double  array2D[10][10] = {0.0};
        bool    logicalarray[2] = {false};
    }

You think that this code initializes MyClass's member variables. What it actually does is declare two local (automatic) variables, initialize them, and then return. The member variables aren't touched, and are hidden within Init()'s body.

You could fix this by:

void Init()
    {
        array2D[10][10] = {0.0};
        logicalarray[2] = {false};
    }

But I would argue that this is technically correct but still wrong. You are using what's called two-phase construction. The two phases are: 1) Construct an MyObject. 2) Initialize the MyObject by calling it's Init() function.

Ugly. Nasty. Prone to errors & forgetfullness. Semantically incorrect because the constructor doesn't leave the object in a fully initialized state. You should, as a general rule, avoid two-phase construction at all costs. A better way is to perform all initialization in the object's constructor -- preferably in a member initialization list, but this is difficult/impossible for aggregate members. In those cases, initialize in the constructor's body:

class MyClass
{
public:
  MyClass ()
  {
    array2D[10][10] = {0.0};
    logicalarray[2] = {false};
  }
  // ...
};

Now, construction of the MyObject also means initialization of MyObject. And after all, that's what constructors are for. Do that instead.

The problem here is that we can't initialize aggregates like this. We can only initialize an aggregate using this syntax at construction time, but that happened in the (non-existant) initialization list. We can't use an initialization list in C++03 to initialize an aggregate. So you're left with an ugly situation:

MyClass ()
{
  memset (logicalarray, logicalarray+2, 0);
  memset (array2D, array2D+sizeof (array2D), 0);
}

This leads me to an argument that you shouldn't be using raw arrays in the first place, but rather a vector (or something) and initialize it via something like:

   class MyClass
    {
    public:
      std::vector <bool> mBools;
      std::vector <std::vector <double> > mDoubles;
      MyClass ()
      {
        std::fill_n (std::back_inserter (mBools), 2, false);
      }
    };

I'll leave initialization of mDoubles as an exercise.

John Dibling
  • 99,718
  • 31
  • 186
  • 324
  • @NeilKirk: Clarify. As opposed to what? – John Dibling Sep 30 '13 at 14:14
  • `{0.0}` and `{false}` it should default to a 0 equivalent value. – Neil Kirk Sep 30 '13 at 14:22
  • @NeilKirk: Yes, correct. You can omit all initializers in an aggregate if what you want is value-initialization. – John Dibling Sep 30 '13 at 14:27
  • @John Dibling So the constructor approach still works in non C++11? – ontherocks Sep 30 '13 at 14:49
  • @ontherocks: It's psudocode. You'll need to work out the actual syntax what stuff you need to `#include`. – John Dibling Sep 30 '13 at 15:02
  • @John Dibling That's what I am saying, this doesn't work in non C++11. It's not because of missing headers, etc. – ontherocks Sep 30 '13 at 15:05
  • @John Dibling : It in the line where array2D has been initialized in the constructor i.e. `MyClass () { array2D[10][10] = {0.0};` – ontherocks Sep 30 '13 at 15:19
  • @ontherocks: Oh, right. Keep reading: "The problem here is that we can't initialize aggregates like this." Follow along further an you'll see that you'll need to either do a `memset` or a `fill` type of thing. – John Dibling Sep 30 '13 at 15:20
  • @ontherocks: The main point there was that initialization should be done in the constructor, not during 2-phase construction. The actual syntax you use to do the initialization is explored towards the latter half of the post. – John Dibling Sep 30 '13 at 15:22