[Development] RFC: RAII for property changes

[Marc Mutz]

On Wednesday 15 April 2015 16:58:24 André Somers wrote:
> Basically, if you have this code (or something like it) somewhere, you 
> already have a problem:
> 
> void MyClass::setFooAndBar(QString foo, int bar)
> {
>    if (m_foo != foo) {
>      m_foo = foo;
>      emit fooChanged(foo);  //this is where things may start to go wrong 
> already
>    }
> 
>    if (m_bar != bar) {
>      m_bar = bar;
>      emit barChanged(bar);
>    }
> }

1. This function has more than one responsibility. Functions with more than
   one responsibility are hard to get right, to maintain, and in general
   hard to make exception-safe.
2. This function doesn't have all-or-nothing / transactional semantics.

That's basically both different way of saying that functions should be 
structured such that they:

1st perform anything that can fail _without_ modifying the state of the 
program
2nd commit the new state with never-fail operations
3rd clean up (this includes notification)

> void MyClass::setFooAndBar(QString foo, int bar)
> {
> 
>    bool fooHasChanged(false);
>    bool barHasChanged(false);
>    
>    if (m_foo != foo) {
>    
>      m_foo = foo;
>      fooHasChanged = true;
>    
>    }
>    
>    if (m_bar != bar) {
>    
>      m_bar = bar;
>      barHasChanged = true;
>    
>    }
>    
>    if (fooHasChanged) emit fooHasChanged(foo);
>    if (barHasChanged) emit barHasChanged(bar); // [...]
> }

This code still doesn't meet that goal. It first modifies m_foo. If the 
assignment to m_bar throws, then m_foo has been changed, but m_bar hasn't.

void MyClass::setFooAndBar(QString foo, int bar)
{
   const bool fooHasChanged = m_foo != foo;
   const bool barHasChanged = m_bar != bar;

   m_foo.swap(foo);
   m_bar = bar;  // int can't throw

   if (fooHasChanged) emit fooHasChanged(m_foo);
   if (barHasChanged) emit barHasChanged(m_bar);
}

Untested code:

#define EMIT_AT_SCOPE_EXIT_WHEN_CHANGED(value, signal) \
  struct Guard##value {
      decltype(value) oldValue;
      decltype(value) &ref;
      ~Guard##value() { if (ref != oldValue) emit signal(ref); }
  } guard##value = { value, value };

For C++98, the type would have to be passed instead of using decltype.
It's still less efficient than it could be, due to oldValue (imagine it being 
a std::vector), but that can be fixed at the cost of more arguments. Maybe a 
lot of this stuff can also be templatised, to effectively get something like:

const GuardBase &valueGuard = guard(value, &signal);

But if the call to signal in this local scope is still indirect, as it is with 
most function pointers, this is still a no-no, imo.

Thanks,
Marc

-- 
Marc Mutz <marc.mutz at kdab.com> | Senior Software Engineer
KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
Tel: +49-30-521325470
KDAB - The Qt Experts