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