views:

81

answers:

4

Hi all,

When I have a method that calls a set of methods that offer strong guarantee, I often have a problem on rolling back changes in order to also have a strong guarantee method too. Let's use an example:

// Would like this to offer strong guarantee
void MacroMethod() throw(...)
{
  int i = 0;
  try
  {
    for(i = 0; i < 100; ++i)
       SetMethod(i); // this might throw
  }
  catch(const std::exception& _e)
  {
    // Undo changes that were done
    for(int j = i; j >= 0; --j)
      UnsetMethod(j); // this might throw
    throw;
  }
}

// Offers strong guarantee
void SetMethod(int i) throw(...)
{
  // Does a change on member i
}

// Offers strong guarantee
void UnsetMethod() throw(...)
{
  // Undoes a change on member i
}

Obviously, the UnsetMethod could throw. In which case, my MacroMathod() only offers basic guarantee. Yet, i did all I could to offer a strong guarantee, but I can't be absolutly sure my UnsetMethod() will not throw. Here's my questions:

  1. Should I even try to offer a strong guarantee in this case?
  2. Should I document my MacroMethod() as having a basic or strong guarantee? Even if it is very unlikely UnsetMethod will throw?
  3. Can you see a way to make this method truly offer a strong guarantee?
  4. I should probably put the call to UnsetMethod() in a try, but that feels rather heavy, and what should I do in the catch?

Thanks!

+1  A: 

If a strong exception guarantee is important to MacroMethod(), I would redesign UnsetMethod() to not throw anything at all, if you can. Of course how this can be done depends on what you're doing.

You're using UnsetMethod() to clean up after a failure from SetMethod(). If UnsetMethod() fails to clean up, what can you do about that? It's the same reason why throwing exceptions from destructors is extremely dangerous.

In silico
+6  A: 

A good pattern to try to achieve this is to make your method work on a copy of the object that you want to modify. When all modifications are done, you swap the objects (swap should be guaranteed not to throw). This only makes sense if copy and swap can be implemented efficiently.

This method has the advantage that you do not need any try...catch-blocks in your code, and also no cleanup-code. If an exception is thrown, the modified copy gets discarded during stack unwinding, and the original was not modified at all.

Space_C0wb0y
My example did not described the full extent of the problem well, not to mention the performance problem this solution would cause: the member is a data array and such operations can be done regularly. But the solution you proposed brings me to the right solution, which is essentially what you propose. I will add a SetCachedMethod() and ApplyCache()/ClearCache() methods. SetCachedMethod() will throw, apply and clear won't. So I essentially have partial object copy instead of a full one. Thanks!
Geeho
@Geeho: Maybe you can make the cache an external object, or bind the calling of clear-cache to some guard (look up [RAII](http://en.wikipedia.org/wiki/Resource_Acquisition_Is_Initialization)). This way you still can prevent having a try...catch block in your code.
Space_C0wb0y
Not at all a bad idea. Will look at that first thing tomorrow morning.
Geeho
+1  A: 
  1. No, you can't in general with the code you gave. However, depending on your problem, maybe you can make a copy of the data, perform SetMethod on that copy and then swap the representation. This provides strong guarantee, but again depends on the problem.
  2. You can document: Strong guarantee if UnsetMethod doesn't throw, basic otherwise. Actually this explains why it is said that destructors should not throw. Actually any undo operations should not throw.
  3. Yes, see 1.
  4. No, it makes no sense.
ybungalobill
A: 

Fundamentally, what you need to do in functions that call throwing functions is work on a temporary copy of the data until all of the possibly-throwing sub functions have finished, then swap (which must not throw) the temporary values in to the 'real' structure.

Some illustrative psudocode:

void MacroMethod() throw(...)
{
  int i = 0;
  MyValues working_copy[100] = *this;  //obviously this is psudocode as I don't know your real code
  for(i = 0; i < 100; ++i)
     working_vopy[i].SetMethod(i); // if this throws the exception will propogate out, and no changes will be made
  swap( *this, working_copy);  // this must not throw
}
John Dibling