Is this bad RAII design?

Question

I come from a Java background, but I learned C++ after and have been programming with it for a few years now (mostly debugging and writing fixes, not designing programs from scratch). However, I ran into an issue today and frankly, I'm a little surprised it took this long to come across it.

Let's say I have a class named Class1 whose header file contains (among other code):

class Class1 {
    private:
        Class2 object;
}

The Class2 class doesn't have a default constructor specified. Now, in the Class1 constructor, I'm reading the binary header of a file and using the info I parse from that to initialize Class2, as shown with the pseudo-code below:

Class1::Class1(std::string) {
    // Read some binary info from a file here

    // Parse that binary info

    object2 = Class2(info);

In Java, since it's not using the RAII paradigm, that would be perfectly legal. However, since C++ uses RAII, object 2 was already initialized with its default constructor by the time I do object2 = Class2(info);. I couldn't have just called that constructor originally (in the Class1 header file) because I didn't have the info I needed to create object yet. However, I can't just make object2 local to the constructor, because I need other functions to be able to see/use it.

Clearly this doesn't work. What's the standard approach for this stuff? I actually thought about just changing Class1 to having a Class2 pointer like so:

class Class1 {
    private:
        Class2* objectPointer;
}

and then calling *objectPointer = Class2(info). However, "Class2" in my case is an ifstream and it seems that the operator= function has been deleted and doesn't work with either approach.

So... how do I do this?

I don't think it's quite right to say that "C++ uses RAII". C++ enables RAII, but doesn't require its use. — Rob K, Sep 13 '12 at 17:36
Note that RAII is a horrible name for a wonderful feature. While the name hints at initialization, the important side is destruction: every resource should be managed by a class. It has nothing for/against 2 phase initialization (although you may want to avoid it whenever possible) — David Rodríguez - dribeas, Sep 13 '12 at 19:51
@RobK Yeah, I probably should have worded my question a bit more carefully... something along the lines of "However, since C++ allows for RAII and it's generally good practice," or something similar. — mtrewartha, Sep 13 '12 at 20:07

score 6 · Accepted Answer · answered Sep 13 '12 at 17:06

because your object is not const, that's all perfectly legal. However, if you want to initialise objects in the initialisation phase, you must provide the information. You can do that such

 Class1::Class1(std::string file_name) : object(InfoFromFile(file_name)) {}

where InfoFromFile() would either be a standalone function (declared in an anonymous namespace within a .cc file) or a static member function of Class1. If more information than file_name is required to generate the information needed for Class2, you can provide it to that function.

Thank you! I had completely forgotten that a huge point of using initialization lists was to choose which constructor is called for that member instead of the default. I added `object` to the list like you mentioned and sure enough, all is well now. — mtrewartha, Sep 13 '12 at 19:59

score 5 · Answer 2 · edited May 23 '17 at 12:03

If the "read and parse" part doesn't require the object to have been constructed, then you could move that to a static (or non-member) function, and the initialise the member using the result of that:

Class1::Class1(std::string) :
    object2(read_class2_info(some_file))
{}

If you really can't separate the file reading from the object construction for some reason, and can't reassign object2 later, then you will need to use a pointer. To create the object, you'd use new:

objectPointer = new Class2(info);

However, to save yourself from having to mess around the Rule of Three, you should avoid managing dynamic objects yourself, and instead use a smart pointer:

std::unique_ptr<Class2> objectPointer;

objectPointer.reset(new Class2(info));

score 2 · Answer 3 · answered Sep 13 '12 at 17:05

I suggest to use a local function to do the bits that read the binary info and directly initialize in the constructor list:

namespace {

InfoObject readInfo(std::string s)
{
// Read some binary info from a file here

// Parse that binary info
  return info;
}

}

Class1::Class1(std::string s)
: object(readInfo(s))
{
}

Using a pointer is, of course, also an option. This is also why this works more naturally in Java (every user type is a pointer, internally). You probably want to use a smart-pointer though.

score 1 · Answer 4 · answered Sep 13 '12 at 17:06

I think you would have to initialize object with dummy data, and then update it with the new data once you've parsed it.

Something like this:

Class1(std::string str)
 : object("some valid-but-meaningless data")
{
    // parse info
    object = Class2(info);

If this is no good, you probably have to do the parsing in a static factor method that then passes info to the constructor (which you might very well make private). Something like this:

Class1(Info info)
 : object(info)
{
    // whatever else you want
}

static Class1 create(std::string)
{
    // parse info
    return Class1(info);
}

EDIT: I actually like Walter's answer better; using a function in the initialization is a good idea. I'm just leaving this here for some alternate ideas you can consider.

Is this bad RAII design?

4 Answers4