Mike
Mike

Reputation: 771

Is this bad RAII design?

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?

Upvotes: 4

Views: 1172

Answers (4)

Mike Seymour
Mike Seymour

Reputation: 254471

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));

Upvotes: 5

KRyan
KRyan

Reputation: 7598

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.

Upvotes: 1

Walter
Walter

Reputation: 45444

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.

Upvotes: 6

ltjax
ltjax

Reputation: 15997

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.

Upvotes: 2

Related Questions