Re: [Inkscape-devel] Why is the position of this line of code so touchy?
On Jan 23, 2014, at 4:34 PM, mathog wrote:
In the files in src/display there are many instances of this
Inkscape::DrawingContext::Save save(dc);
First thing, I don't understand the syntax. What does the lower case
"save" in that line indicate? The only places "::Save save(" appear
in
the source code are instances
of this line.
Well... the first problem is that the class has a bad name. That is complicating things a
bit. The 'Save' class has a verb for a name, whereas we should use nouns for names
of classes. Looking into the sources, it seems to be an attempt to merely wrap a call to
save paired with a later call to restore. So... just a simple attempt to tune the name
would give us "Saver" or perhaps "ContextSaver" instead of
"Save".
Inkscape::DrawingContext::ContextSaver saver(dc);
dc.transform(aff);
double thickness = decorateItem(dc, aff, phase_length);
dc.setLineWidth(thickness);
dc.strokePreserve();
dc.newPath();
Then whenever the 'saver' instance goes out of scope, either by exiting a closing
'}' brace or by an exception being thrown, the call to cairo_restore will be
invoked.
However... as we ponder that explanation, a potentially better name presents itself. Could
this make the code clearer at the point of use?
{
Inkscape::DrawingContext::ContextRestorer contextRestorer(dc);
dc.transform(aff);
double thickness = decorateItem(dc, aff, phase_length);
dc.setLineWidth(thickness);
dc.strokePreserve();
dc.newPath();
}
Then by its name we are given a clue that the *reason* for the class to be around is to
restore things. That we first have to save them becomes a secondary implementation detail,
where as the "restore" aspect is exposed as the primary use.