CODEWITHSUNDEEP
c++constructorraii
anjruu
anjruu

Reputation: 1264

RAII way to choose between two constructors

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?

Upvotes: 2

Views: 237

Answers (5)

Simple
Simple

Reputation: 14390

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;

Upvotes: 0

James Kanze
James Kanze

Reputation: 153929

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;

Upvotes: 0

rubenvb
rubenvb

Reputation: 76539

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.

Upvotes: 0

Kerrek SB
Kerrek SB

Reputation: 477110

Maybe like this:

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

Upvotes: 4

Paul Evans
Paul Evans

Reputation: 27577

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.

Upvotes: 0

Related Questions