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

[Saam Barati]

Right. I see what you're saying. The name doesn't confuse me with respect to 
these semantics but I see that's it's subtly wrong.

The use case I was thinking of is this:
`
class Foo {
Foo() : m_change(someIntVariable, 20) { }
...
...
ScopedChange m_change;
};
`

Which is admittedly an odd use case that probably doesn't exist in WebKit. 
However, if this use case were common, the name ScopedChange feels wrong for 
it. 

- Saam

> On Dec 25, 2016, at 7:29 PM, Maciej Stachowiak  wrote:
> 
> 
>> On Dec 25, 2016, at 11:35 AM, Saam Barati  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  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  
 wrote:
 
 On Fri, 2016-12-23 at 05:42 +, 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@lists.webkit.org
 https://lists.webkit.org/mailman/listinfo/webkit-dev
>>> 
> 
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Antti Koivisto]

On Mon, Dec 26, 2016 at 5:29 AM, Maciej Stachowiak  wrote:

>
> ScopedChange(tachyonFlux, 2.0);
>

I wish there was a compiler warning against this. I have caused at least
one bug by doing this exact thing.

(a temporary gets deleted at the end of the expression)


   antti
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev