[webkit-dev] Naming preference: SetForScope vs. TemporaryChange

[Maciej Stachowiak]

> On Dec 25, 2016, at 11:35 AM, Saam Barati <sbarati at apple.com> wrote:
> 
> I like ScopedChange the most out of all the names. It's a bit unfortunate that such a name doesn't work well in the context of having a ScopedChange as a member variable. I think TemporaryChange works much better as a name if the use is as a member variable.
> 
> My hunch would be the most frequent use of this class is as a scoped change. If almost all of the uses are indeed for this, my name preference would be:
> 1. ScopedChange
> 2. TemporaryChange
> 
> If there are a lot of uses where the class isn't used as a scoped change, my preference would be to revert to TemporaryChange.

I'm not sure what distinction you are trying to draw between "scoped change" and "temporary change". It seems pretty clear that this class is meant to be used as a value object on the stack, so anything it does is scoped.

The distinction I was trying to draw is that, if you make additional changes to the value still within the scope of the object, they will be overwritten. That aspect isn't really captured by either "ScopedChange" or "TemporaryChange". That's because, in some fundamental underlying sense, what's really scoped is the restore of the original value, not the change. 

Maybe a concrete example would help show what I'm taking about:

// global
double tachyonFlux = 3.14;

void redirectEnginesToShield()
{
	ScopedChange(tachyonFlux, 2.0);

	doOneThing();

	tachyonFlux = 1.0;

	doAnotherThing();

	// on scope exit, both the scopedChange and the explicit assignment are undone
}

Perhaps this doesn't matter in practice because there's never actually additional assignments in the scope of one of these things so it will be clear enough as actually used.

Regards,
Maciej


> 
> - Saam
> 
>> On Dec 23, 2016, at 6:50 AM, Maciej Stachowiak <mjs at apple.com> wrote:
>> 
>> 
>> A few more coats of paint for the bike shed:
>> 
>> It's a little unusual to have a class name that's a verb phrase instead of a noun phrase. And in this case if you interpret "Set" as a noun you'll get entirely the wrong idea. Some alternatives that avoid this, but has the better clarity of "Scope" instead of "Temporary" would be "ScopedChange or "ScopedAssignment".
>> 
>> One additional thing to think about: the class doesn't just have the effect of limiting the assignment to a scope. It will also undo any further assignments to the reference it holds that happen until it is destroyed. Save-restore semantics like this are common but often the names involved highlight the restore rather than the setting. I can't think of a great name off the top of my head but something like RestoreOnScopeExit seems more technically accurate than SetForScope.
>> 
>> - Maciej
>> 
>>> On Dec 23, 2016, at 6:32 AM, Michael Catanzaro <mcatanzaro at igalia.com> wrote:
>>> 
>>> On Fri, 2016-12-23 at 05:42 +0000, Yusuke SUZUKI wrote:
>>>> Personally I like the name "SetForScope" since the name "scope"
>>>> states that this value change is tied to C++ scope.
>>> 
>>> Me too. The name is pretty clear. The first time I saw TemporaryChange
>>> I had to look at the implementation to see what it did.
>>> 
>>> Michael
>>> _______________________________________________
>>> webkit-dev mailing list
>>> webkit-dev at lists.webkit.org
>>> https://lists.webkit.org/mailman/listinfo/webkit-dev
>>