Oliver Schönrock
Oliver Schönrock

Reputation: 1156

constructor initialisation order when Base constructor depends on reference from Derived

I have a base class which writes objects to a std::ostream with buffering. I want to call this with

  Obj obj;
  flat_file_stream_writer<Obj> writer(std::cout);
  writer.write(obj);
  writer.flush();

but also with

  Obj obj;
  flat_file_writer<Obj> writer("filename.bin");
  writer.write(obj);
  writer.flush();

The latter call must establish a std::ofstream instance, and hold that for the duration of writing, and can then call the former version.

I thought simple non-virtual inheritance would be fine here. But I have constructor initialization order issues.

warning: field 'ofstream_' will be initialized after base
      'flat_file_stream_writer<hibp::pawned_pw>' [-Wreorder-ctor]

Because the base class depends on a reference to the object held by the derived this seems like chicken and egg and can't be solved?

Is inheritance just the wrong abstraction here? A clean alternative?

template <typename ValueType>
class flat_file_stream_writer {

  public:
    explicit flat_file_stream_writer(std::ostream& os, std::size_t buf_size = 100)
        : db_(os), buf_(buf_size) {}

    void write(const ValueType& value) {
        if (buf_pos_ == buf_.size()) flush();
        std::memcpy(&buf_[buf_pos_], &value, sizeof(ValueType));
        ++buf_pos_;
    }

    void flush() {  // could also be called in destructor?
        if (buf_pos_ != 0) {
            db_.write(reinterpret_cast<char*>(buf_.data()), // NOLINT reincast
                      static_cast<std::streamsize>(sizeof(ValueType) * buf_pos_));
            buf_pos_ = 0;
        }
    }

  private:
    std::ostream&          db_;
    std::size_t            buf_pos_ = 0;
    std::vector<ValueType> buf_;
};

template <typename ValueType>
class flat_file_writer : public flat_file_stream_writer<ValueType> {
  public:
    explicit flat_file_writer(std::string dbfilename)
        : dbfilename_(std::move(dbfilename)), dbpath_(dbfilename_),
          ofstream_(dbpath_, std::ios::binary), flat_file_stream_writer<ValueType>(ofstream_) {
        if (!ofstream_.is_open())
            throw std::domain_error("cannot open db: " + std::string(dbpath_));
    }

  private:
    std::string                        dbfilename_;
    std::filesystem::path              dbpath_;
    std::ofstream                      ofstream_;
};

Upvotes: 0

Views: 78

Answers (1)

Oliver Sch&#246;nrock
Oliver Sch&#246;nrock

Reputation: 1156

You can put std::ofstream ofstream_; in a separate struct, then have flat_file_writer inherit from that struct using private. Base classes are initialized in the order they are declared, so make sure to inherit from that struct before flat_file_stream_writer. You can now initialize that ofstream_ before the base class flat_file_stream_writer.

On closer inspection, I think you'll probably want to put all 3 of those members in the struct.

See below for updated code.

So this in effect means that the 3 members in ofstream_holder now come before the flat_file_stream_writer in the layout of flat_file_writer.

This seems like the right solution, not just because it "squashes the compiler warning", but because now we are really getting the correct initialization order, ie construct the std::ofstream first and then pass a reference to it to flat_file_stream_writer. And during destruct the std::ofstream will be destroyed last.

The original code above had this exactly the wrong way around, because the order we write in the derived constructor initializer is ignored and the actual order implemented is layout order, ie the order the members are declared.

(despite the flaws of the original code, it "ran fine" for me with sanitizers etc... but was probably UB?).

template <typename ValueType>
class flat_file_stream_writer {

  public:
    explicit flat_file_stream_writer(std::ostream& os, std::size_t buf_size = 100)
        : db_(os), buf_(buf_size) {}

    void write(const ValueType& value) {
        if (buf_pos_ == buf_.size()) flush();
        std::memcpy(&buf_[buf_pos_], &value, sizeof(ValueType));
        ++buf_pos_;
    }

    void flush() {
        if (buf_pos_ != 0) {
            db_.write(reinterpret_cast<char*>(buf_.data()), // NOLINT reincast
                      static_cast<std::streamsize>(sizeof(ValueType) * buf_pos_));
            buf_pos_ = 0;
        }
    }

  private:
    std::ostream&          db_;
    std::size_t            buf_pos_ = 0;
    std::vector<ValueType> buf_;
};

struct ofstream_holder {
    explicit ofstream_holder(std::string dbfilename)
        : dbfilename_(std::move(dbfilename)), dbpath_(dbfilename_),
          ofstream_(dbpath_, std::ios::binary) {}

    std::string           dbfilename_;
    std::filesystem::path dbpath_;
    std::ofstream         ofstream_;
};

template <typename ValueType>
class flat_file_writer : private ofstream_holder, public flat_file_stream_writer<ValueType> {
  public:
    explicit flat_file_writer(std::string dbfilename)
            : ofstream_holder(std::move(dbfilename)), flat_file_stream_writer<ValueType>(ofstream_) {
        if (!ofstream_.is_open())
            throw std::domain_error("cannot open db: " + std::string(dbpath_));
    }
};



Upvotes: 1

Related Questions