C++ – Is the RAII wrapper a good idea for this C transactions API or should I stick to the C style

craii

Consider the following C API:

void BeginTransaction(State *s);
void AddToTransaction(State *s, Object *value);
void CommitTransaction(State *s);

void Foo(State *s, Object *value)
{
    BeginTransaction(s);
    AddToTransaction(s, value);
    CommitTransaction(s);
}

I created a C++ class to wrap this API:

class StateTransaction
{
public:
    StateTransaction(State *s) : state_(s)
    {
        BeginTransaction(state_);
    }

    StateTransaction(const StateTransaction &st) = delete;

    void Add(Object *value)
    {
        AddToTransaction(state_, value);
    }

    ~StateTransaction()
    {
        CommitTransaction(state_);
    }

private:
    State *state_;
}

void Foo(State *s, Object *value)
{
    StateTransaction st(s);
    st.Add(value);
    // When st goes out of scope, the transaction automatically gets committed
}

Assuming that none of the functions can fail, is this a good use case for RAII? My main problem with it right now is that in order to force the transaction to be committed before some other code, I need to put the object in a new scope:

void Foo(State *s, Object *value)
{
    {
        StateTransaction st(s);
        st.Add(value1);
    }

    // Code which relies on transaction being completed
}

And of course, there's the danger that someone will come along and delete the braces because they look useless. That could be fixed with a comment like /* DO NOT DELETE BRACES */, but at that point it seems safer to just use the C API.

So, is the RAII wrapper a good idea? Or should I stick to the C style?

Best Answer

In general using RAII to implement transactions is a good idea. However, you have to be careful when doing anything in the destructor.

  • A destructor cannot return a value indicating success/failure.
  • The destructor really should not throw exceptions, as the destructor may be invoked during exception handling.

This means we cannot really use destructors to represent operations that can fail.

Because the destructor will be invoked during normal control flow and during exception handling, it could happen that the transaction is not in fact complete:

StateTransaction transaction;
transaction.Add(x);
throw "oops";
transaction.Add(y);

Your current design would happily commit the "x" value, and miss the "y" value. This isn't a very transactional transaction, if the transaction may be incomplete.

Destructor-based transactions are more useful if they are implemented as rollbacks – the destructor will roll back the transaction unless it is explicitly committed. Implicit committing doesn't quite work.

To auto-commit only when no exception occurred, use normal control flow. If you want to avoid the possibility that the control flow exited normally but the programmer forgot to call Commit, then a callback-based solution would work (C++11):

template<class TransactionBuilder>
void transaction(TransactionBuilder callback) {
  State transaction;
  BeginTransaction(&transaction);
  callback(&transaction);
  CommitTransaction(&transaction);
}

transaction([&](State* transaction) {
  AddToTransaction(transaction, x);
  throw "oops";
  AddToTransaction(transaction, y);
});
Related Topic