tags:

views:

74

answers:

2

I have an STL container and I need to perform an action on each element in the container. But if the action fails on any element, I want to reverse the action on any elements that have already been changed.

For example, if I had an STL vector with pointers to a number bankAccount classes and wanted to increase each one by $50. But if any of the bank accounts fail to increase by 50, I want to cancel the increase entirely and decrease by $50 any of the accounts that have already been increased.

std::vector<bankAccount*> bankAccounts;
std::vector<bankAccount*>::iterator iter;

for (iter = bankAccounts.begin(); iter != bankAccounts.end(); ++iter)
{
    try
    {
        iter->increaseBalance(50);
    }
    catch (...)
    {
        // One of the bankAccounts failed to increase by 50, now I need to go 
        // back and decrease by 50 all of the bankAccounts that have already 
        // been increased.
    }
}

Is there any elegant way to do this? Maybe with STL algorithms or using reverse iterators?

+9  A: 

Here's what I would do:

  • Move the try/catch outside the loop
  • Create a duplicate of the bankAccounts container
  • Iterate over the duplicate container, calling increaseBalance on each item
  • If the loop sucessfully completed, swap() the original and the duplicate container

The code would look something like this:

std::vector<bankAccount> bankAccounts;
...
std::vector<bankAccount> tmp(bankAccounts);

try
{
   for (iter = tmp.begin(); iter != tmp.end(); ++iter)
   {
     iter->increaseBalance(50);
   }
   bankAccounts.swap(tmp);
}
catch (...)
{
}

Please note that holding a pointer to an object inside a std::vector is generally not that good an idea as the container expects the data stored in it to have value semantics, not pointer semantics. This can lead to dangling pointers, memory leaks and also requires additional cleanup code that you don't need otherwise (to delete the items in container manually). With the code above, I've switched to holding the data inside the vector, if that's not an option you need to ensure that you're using a manual deep copy when you're copying the vector.

Actually, you can reduce the code to the following if you assume the same definitions for bankAccounts and tmp:

std::for_each(tmp.begin(), tmp.end(),
              std::mem_fun_ref(&bankAccount::increaseBalance, 50));
bankAccounts.swap(tmp);

The main advantage of the code above is that in both cases, it is exception safe without any further special handling.

Timo Geusch
Replace the `for` with `for_each` and the pointer call with `mem_fun`
wheaties
You don't even need the try/catch actually unless you actually want to do something else. If increaseBalance fails anywhere the swap simply won't happen. And yeah, I'd switch to for_each as well. +1 though.
Noah Roberts
Yes, the `try/catch` is not really necessary in this context. I'd normally leave it out completely but I also tried to restructure the OP's code while keeping most of it.
Timo Geusch
`std::vector<Account> tmp; std::transform( acc.begin(), acc.end(), std::back_inserter(tmp), boost::bind( std::swap( tmp, acc ); `
David Rodríguez - dribeas
Thanks for the solution. Looks like the fundamental issue is that I need to attempt to perform the action on all elements before I save the result.Why is the code at the end of your solution exception safe?
C Nielsen
@C Nielsen: In case an exception is thrown during the `for_each`, the original container remains in the consistent state it was in before. std::vector::swap supports the nothrow exception guarantee (it most likely only swaps a few internal pointers between the two vectors) so if you've made it to the swap your updated container again is in a consistent state.
Timo Geusch
@C Nielsen: in fact, the `for` loop was exception safe too.
Matthieu M.
+1  A: 

I think a more elegant approach would to treat the operation as a transaction. In other words, create a replacement copy of the accounts, and overwrite the original on success.

Inverse