views:

122

answers:

4

I have some code to update a database table that looks like

try
{
   db.execute("BEGIN");
   // Lots of DELETE and INSERT     
   db.execute("COMMIT");
}
catch (DBException&)
{
   db.execute("ROLLBACK");
}

I'd like to wrap the transaction logic in an RAII class so I could just write

{
   DBTransaction trans(db);
   // Lots of DELETE and INSERT
}

but how would I write the destructor for it?

+1  A: 

Easiest way I can think of would be to set a private member variable in the class in the exception, and test it / perform the appropriate action in the destructor.

tzaman
+3  A: 

You might use the following logic:

  1. Add a commit_done boolean value initialized to false to your Transaction class.
  2. In your constructor, "begin" the transaction.
  3. Add a method to "commit" the transaction and update commit_done accordingly.
  4. In your destructor, call "rollback" only if commit_done is still false
ereOn
+10  A: 

Use following:

transaction tr(db);
...
tr.commit();

When tr.commit() completes it sets the state to "commit done" and destructor does nothing, otherwise it rollbacks.

Checking for exception is bad idea, consider:

transaction tr(db);
...
if(something_wrong)
   return; // Not throw
...
tr.commit();

In this case you probably expect rather rollback then commit but commit would be done.

Edit: but if you still want it badly, take a look on std::uncaught_exception() but read this first http://www.gotw.ca/gotw/047.htm

Artyom
+1 That's how such things are done. There's one problem though: what if you forget to call commit()?
sharptooth
And what if you forget to create transaction variable? You can't prevent all mistakes.
Tadeusz Kopec
@sharptooth: what if you forgot to make the changes that you wanted to commit in the first place? I don't think there's much you can do to protect against incompetence.
jalf
@sharptooth: if you forget to call `commit` the change will be rolled back, and you will notice in the very first test you run and it will be something easy to track. It is much more probable that errors in the common path are diagnosed and corrected that in exceptional cases.
David Rodríguez - dribeas
+1  A: 

By removing the exception handling, you are crippling your RAII.

The code should be

try
{
   DBTransaction trans(db) ;

   // Lots of DELETE and INSERT
   // should one fail, a DBTransactionRollback exception will be thrown

   trans.commit() ;
}
catch(const DBTransactionRollback & e)
{
   // If really needed, you could extract failure information from "e"
}

The differences with your original code are what motivated my answer:

  1. There is nothing needed in the "catch" : The destructor will assume an automatic rollback unless the commit() method was called with success (which could, for example, set some private boolean member of DBTransaction to true). The catch is where the code will continue, assuming the transaction failed.

  2. You should create a dedicated exception (I named it DBTransactionRollback) to throw the moment something fails in one of your commands. Thus, the catch will only catch transaction rollback-motivated exception, and not other exceptions (like STL, etc.)

  3. The use of the exception mechanism enables you to put your code in multiple functions, called from this try/catch block of code, without having to deal with boolean returns and other error code returns.

Hope this answers your question.

paercebal