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.m...@kdab.com> | Senior Software Engineer
KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
Tel: +49-30-521325470
KDAB - The Qt Experts
_______________________________________________
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
