Il wrapper RAII è una buona idea per questa API di transazione C o dovrei attenermi allo stile C?

In generale, utilizzare RAII per implementare le transazioni è una buona idea. Tuttavia, devi fare attenzione quando fai qualcosa nel distruttore.

• Un distruttore non può restituire un valore che indica successo / fallimento.
• Il distruttore in realtà non dovrebbe generare eccezioni, in quanto il distruttore può essere invocato durante la gestione delle eccezioni .

Questo significa che non possiamo davvero usare i distruttori per rappresentare operazioni che possono fallire.

Poiché il distruttore verrà richiamato durante il normale flusso di controllo e durante la gestione delle eccezioni, potrebbe accadere che la transazione non sia effettivamente completata:

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

Il tuo progetto attuale si impegna felicemente nel valore "x" e manchi il valore "y". Questa non è una transazione molto transazionale, se la transazione può essere incompleta.

Le transazioni basate su Destructor sono più utili se implementate come rollback: il distruttore eseguirà il rollback della transazione a meno che non sia esplicitamente impegnato. L'impegno implicito non funziona del tutto.

Per eseguire il commit automatico solo quando non si sono verificate eccezioni, utilizzare il normale flusso di controllo. Se vuoi evitare la possibilità che il flusso di controllo esca normalmente, ma il programmatore ha dimenticato di chiamare Commit , allora funzionerebbe una soluzione basata sul callback (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);
});

L'utilizzo di BeginTransaction e EndTransaction direttamente può dare risultati diversi rispetto a RAII.

Se confrontiamo:

BeginTransaction(s);
AddToTransaction(s, value);
CommitTransaction(s);

A:

{
    StateTransaction st(s);
    st.Add(value1);
}

C'è una differenza significativa se il passaggio di aggiungere qualcosa alla transazione genera un'eccezione. Nel primo caso, il lancio significa che l'esecuzione in quell'ambito si interromperà, le risorse saranno ripulite e si riavvieranno dal gestore di eccezioni corrispondente più vicino.

Nel caso RAII, poiché la transazione è una risorsa creata in tale ambito, verrà distrutta prima di uscire da tale ambito.

Quindi, se il primo AddToTransaction genera un'eccezione, la transazione verrà lasciata aperta, ma nel secondo verrà confermata.

Questo probabilmente non fa molta differenza se aggiungi una sola voce alla transazione, ma considera una transazione in cui aggiungiamo diversi elementi:

BeginTransaction(s);
AddToTransaction(s, value1);
AddToTransaction(s, value2);
AddToTransaction(s, value3);
AddToTransaction(s, value4);
CommitTransaction(s);

Ora, cosa succede se (ad esempio) si aggiunge il valore 4 ai lanci della transazione? Non abbiamo fatto nulla per chiudere la transazione: abbiamo una transazione aperta con valori 1-3 solo "sospesi".

In questo caso, probabilmente vorremmo abortire la transazione, non impegnarla però. Quindi il dtor dovrebbe probabilmente assomigliare a questo:

~StateTransaction()
{
    if (std::uncaught_exceptions() > 0)
        AbortTransaction(state);
    else
        CommitTransaction(state_);
}

In questo modo sappiamo che la transazione sarà sempre chiusa, ma ha almeno un po 'di informazioni sull'abbandono o il commit della transazione.

Se scriviamo codice per fare lo stesso utilizzando CommitTransaction / AbortTransaction direttamente, finisce in modo piuttosto disordinato. Di solito finiamo con qualcosa di simile:

try {
    BeginTransaction()
    AddToTransaction()
    AddToTransaction()
    CommitTransaction();    
}
catch (...) {
    AbortTransaction();
}

Se proviamo a gestire questo aspetto senza eccezioni, la vita peggiora ancora di fretta. In genere finiamo con nidificazione profonda:

if (BeginTransaction())
    if (AddToTransaction())
        if (AddToTransaction())
            CommitTransaction();
        else
            AbortTransaction();
    else
        AbortTransaction();

Penso che un blocco senza una ragione ovvia per l'esistenza sia (di gran lunga) la soluzione più pulita che funzioni realmente.

Per quanto riguarda il tuo problema reale, supponendo che "CommitTransaction" sia mal chiamato e debba essere chiamato anche in casi eccezionali (secondo i tuoi commenti) e il tuo problema è con l'ambito di RAII, vedo due scelte.

Opzione 1: Commenta appropriatamente l'ambito. Sintassi simile a C # (C # ha "usato" per il suo RAII), o semplicemente un messaggio sull'ambito. Scelta stilistica, basta essere coerenti.

void Foo(State* s, Object *value)
{
    //using(StateTransaction st) 
    {
        StateTransaction st(s);
        st.Add(value);
    }
    // Code which relies on transaction being completed
}

o

void Foo(State* s, Object *value)
{
    //scope for StateTransaction st
    {
        StateTransaction st(s);
        st.Add(value);
    }
    // Code which relies on transaction being completed
}

Opzione 2: Riforma il tuo ambito in un metodo privato / funzione globale aggiuntiva.

void FooAddValue(State* s, Object *value)
{
    StateTransaction st(s);
    st.Add(value);
}
void Foo(State* s, Object *value)
{
    FooAddValue(s, value);
    // Code which relies on transaction being completed
}

Preferisco l'opzione 2. Il fatto che tu ritieni necessario questo sotto-ambito indica che la tua funzione ha più responsabilità e viola quindi SOLID principio di Single Responsibility e dovrebbe essere suddiviso in più funzioni.

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

Considera un wrapper RAII che esegue il rollback della transazione in destructor, con commit esplicito (nota: ho modificato l'API per utilizzare i riferimenti:

class StateTransaction
{
public:
    StateTransaction(State &state) : state_(s), active_{ true }
    {
        BeginTransaction(&state_);
    }

    ~StateTransaction()
    {
        if(active_)
        {
            RollbackTransaction(&state_); // you didn;t mention this API,
                                         // but your C transactional 
                                         // layer should have something like this
        }
    }

    void Commit() noexcept
    {
        active_ = false;
        CommitTransaction(&state_);
    }

    void Add(Object &value) { ... }

private:
    bool active_;
    State &state_;
};

Codice cliente:

void Foo(State &s, Object &value)
{
    StateTransaction transaction{ s };
    transaction.Add(value);
    // ... may have multiple steps here
    transaction.Commit();
}

Se qualcosa getta, (nei vari passaggi della transazione), il transaction.Commit () non verrà raggiunto e la transazione verrà automaticamente ripristinata in caso di distruzione.

Riguardo all'ambito artificiale (e qualcuno lo rimuove per errore), basta chiamarlo e documentarne il motivo (invece di documentare ciò che i tuoi colleghi dovrebbero fare con esso):

void FooTransaction(State & s, Object & value) // name tells you it's a transaction (and
                                             // supposed to be one)
                                             // if it doesn't add a comment about it
{
    StateTransaction transaction{ s };
    transaction.Add(value);
    // ... may have multiple steps here
    transaction.Commit();
}

Detto questo, un wrapper RAII potrebbe non essere la migliore astrazione di primo livello da utilizzare. Prendi in considerazione l'aggiunta della seguente API su di essa:

void transact(State& s, std::vector<Object*> values)
{
    StateTransaction transaction{ s };
    for(auto& x: values)
        transaction.Add(*x);
    transaction.Commit();
}

Codice cliente:

transact(&state, { value });

Ciò rende chiaro ciò che stai facendo e che tutti gli effetti (come la transazione in corso di commit) sono stati completati al termine dell'esecuzione della funzione.