On Feb 28, 2014, at 3:22 PM, mathog wrote:
On 28-Feb-2014 14:32, Jon Cruz wrote:
> On Jan 23, 2014, at 4:34 PM, mathog wrote:
>> Inkscape::DrawingContext::Save save(dc);
> Inkscape::DrawingContext::ContextSaver saver(dc);
Yes, that is less confusing than the original.
That said, the construct is maintenance programmer hostile, since it is
hiding something important for not much in return. This appears to be
OOP anathema, because the second action must be explicitly coded, but
for me the code would be much clearer it it just did the explicit:
Except that is C and not C++. Also, that does not help in the cases where an exception or
early return (something I dislike) is encountered before the explicit cairo_restore(ct)
call is made.
I think it is just that exceptions and RAII are common C++ approaches that one working in
C++ needs to learn and recognize. That said, the names involved can definitely help out in
order to signal more explicitly when RAII is in use.
To help with people unfamiliar with the RAII paradigm, I'll add some more explicit and
visible info to our wiki, etc.
I understand (now) what Inkscape::DrawingContext::Save does, but it
some time to figure it out. Conversely, the cairo save/restore pair is
fundamental, and that pair is commonly used in many other places in
Inkscape (and hundreds of other programs). If it must be done with the
existing methodology, the construct should say clearly what it does:
Definitely something to keep in mind as we clean things up. In this case it looks like the
code itself is good, and that it only needed more explicit naming to make its intent