RAII way to choose between two constructors

Question

I have a class that holds a large table of data, with a constructor that takes all of the parameters needed to calculate that data. However, it takes a long time to run, so I've added a constructor that takes a stream, and reads the data in from that stream. I'm having trouble coming up with a RAII way of designing this class though, since I have two constructors, and at run time I need to choose between them. This is what I've come up with:

std::string filename; // Populated by command line arguments
DataTable table; // Empty constructor, no resource acquisition or initialization

if( filename.empty() ) {
    table = DataTable(/*various parameters*/);
} else {
    std::ifstream filestream(filename);

    table = DataTable(filestream); // Reads from file
}

That looks pretty fragile to me. The default constructor will leave the object in a valid state, but a useless one. The only use of it is to create a "temporary" object in the outer scope, to be assigned to in one of the branches of the if statement. Additionally, there's a flag "inited" behind the scenes to manage if the object was default-constructed or fully initialized. Is there a better way to design this class?

Answer 1

Maybe like this:

DataTable foo = filename.empty()
              ? DataTable(x, y, z)
              : DataTable(std::ifstream(filename));

Answer 2

Move the file test code that decides which way to init into the ctor, move the ctors into two private init functions, call one of these from the ctor or throw an exception if everything fails.

Answer 3

Some thoughts:

1. Get rid of the "inited" flag.
2. Get rid of the default constructor if it can't sensibly construct the object

3. use this kind of construct to get you a DataTable:

   DataTable get_me_my_data_fool(ParameterTypes... params, const string& filename = "")
   {
     if(!filename.empty())
       return DataTable(std::ifstream(filename)); // check if file exists!
     else  
       return DataTable(params...);
   }
   

Actually, now that I think about it, it would be better to just put this logic into the DataTable constructor.

Answer 4

If the class supports copy, then Kerrek SB's solution is the way to go. From what you say, however, copying is expensive. In that case, and you can use C++11, you might try adding a move constructor, in order to avoid the deep copy. Otherwise, you're probably stuck allocating dynamically:

std::auto_ptr<DataTable> fooPtr( filename.empty()
                                 ? new DataTable( x, y z )
                                 : new DataTable( filename ) );
DataTable& foo = *fooPtr;

Answer 5

Here's another idea for completeness sake:

template<typename T>
class uninitialised
{
public:
    ~uninitialised()
    {
        if (alive_) {
            operator T&().~T();
        }
    }

    template<typename... Ts>
    void create(Ts&&... args)
    {
        assert(!alive_ && "create must only be called once");
        void* const p = obj_;
        ::new(p) T(std::forward<Ts>(args)...);
        alive_ = true;
    }

    operator T&()
    {
        assert(alive_ && "T has not been created yet");
        return *reinterpret_cast<T*>(obj_);
    }

private:
    bool alive_ = false;
    alignas(T) unsigned char obj_[sizeof(T)];
};

// ...

std::string filename;
uninitialised<DataTable> table;

if (filename.empty()) {
    table.create(/* various parameters */);
} else {
    std::ifstream filestream(filename);
    table.create(filestream);
}

DataTable& tbl = table;