Re: [webkit-dev] For your consideration: Naming scheme for fooIfExists/ensureFoo

2019-02-21 Thread Ryosuke Niwa
For those who joined webkit-dev after June 2013, see
https://lists.webkit.org/pipermail/webkit-dev/2013-June/thread.html#25056

On Thu, Feb 21, 2019 at 7:54 PM Ryosuke Niwa  wrote:

> I guess we never remembered to update our style guideline back in 2013.
>
> I've uploaded a patch to do this:
> https://bugs.webkit.org/show_bug.cgi?id=194930
>
> - R. Niwa
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] For your consideration: Naming scheme for fooIfExists/ensureFoo

2019-02-21 Thread Ryosuke Niwa
I guess we never remembered to update our style guideline back in 2013.

I've uploaded a patch to do this:
https://bugs.webkit.org/show_bug.cgi?id=194930

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


Re: [webkit-dev] For your consideration: Naming scheme for fooIfExists/ensureFoo

2013-07-03 Thread Bem Jones-Bey
On Jul 2, 2013, at 17:06, Maciej Stachowiak 
m...@apple.commailto:m...@apple.com wrote:


On Jul 1, 2013, at 7:31 PM, Brady Eidson 
beid...@apple.commailto:beid...@apple.com wrote:


On Jul 1, 2013, at 5:27 PM, Ryosuke Niwa 
rn...@webkit.orgmailto:rn...@webkit.org wrote:

I concur.  Maybe
StyleResolver* styleResolverIfExists()
StyleResolver styleResolver()
?

I concur with this.

For this entire discussion, this is where I was hoping it was headed.

I like. Maybe we should use this naming pattern and type signature even when 
only one or the other variant exists?

FWIW, I like this as well. I also like the idea of being consistent with this 
style even when only one exists. Is there a good reason why we wouldn't want to 
do that? I can only think of benefits.

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


Re: [webkit-dev] For your consideration: Naming scheme for fooIfExists/ensureFoo

2013-07-02 Thread Maciej Stachowiak

On Jul 1, 2013, at 7:31 PM, Brady Eidson beid...@apple.com wrote:

 
 On Jul 1, 2013, at 5:27 PM, Ryosuke Niwa rn...@webkit.org wrote:
 
 I concur.  Maybe
 StyleResolver* styleResolverIfExists()
 StyleResolver styleResolver()
 ?
 
 I concur with this.
 
 For this entire discussion, this is where I was hoping it was headed.

I like. Maybe we should use this naming pattern and type signature even when 
only one or the other variant exists?

Regards,
Maciej




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


Re: [webkit-dev] For your consideration: Naming scheme for fooIfExists/ensureFoo

2013-07-01 Thread Ryosuke Niwa
On Sun, Jun 30, 2013 at 9:39 PM, Filip Pizlo fpi...@apple.com wrote:

 On Jun 19, 2013, at 9:41 AM, Dan Bernstein m...@apple.com wrote:

 
 
  On Jun 19, 2013, at 7:37 PM, Timothy Hatcher timo...@apple.com wrote:
 
  What about?
 
  StyleResolver* existingStyleResolver()
  StyleResolver styleResolver()
 
  I like it.
 
 
  — Timothy Hatcher
 
 
  On Jun 19, 2013, at 11:49 AM, Balazs Kelemen kbal...@webkit.org
 wrote:
 
  For me optional seems very misleading and generally different prefixes
 suggests that those objects are not the same.
  Maybe IfExists does not sound nicely but at least it's clear. I would
 choose to have a pointer version with IfExists and a reference version
 which is a noun, like:
 
  StyleResolver* styleResolverIfExists()

 I like this more. I like that the use of 'if' in the name alerts me to the
 fact that the function will return something conditionally.

 By contrast, existingFoo only makes sense to me if I'm already aware of
 the idiom. And although I probably will *become* aware of the idiom it will
 still nonetheless be one of many idioms that I have to be aware of, so I
 fear that I'll forget why Foo is qualified with existing. That's why I
 like fooIfExists - its super explicit about what is going on.


I concur.  Maybe
StyleResolver* styleResolverIfExists()
StyleResolver styleResolver()
?

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


Re: [webkit-dev] For your consideration: Naming scheme for fooIfExists/ensureFoo

2013-07-01 Thread Brady Eidson

On Jul 1, 2013, at 5:27 PM, Ryosuke Niwa rn...@webkit.org wrote:

 On Sun, Jun 30, 2013 at 9:39 PM, Filip Pizlo fpi...@apple.com wrote:
 On Jun 19, 2013, at 9:41 AM, Dan Bernstein m...@apple.com wrote:
 
 
 
  On Jun 19, 2013, at 7:37 PM, Timothy Hatcher timo...@apple.com wrote:
 
  What about?
 
  StyleResolver* existingStyleResolver()
  StyleResolver styleResolver()
 
  I like it.
 
 
  — Timothy Hatcher
 
 
  On Jun 19, 2013, at 11:49 AM, Balazs Kelemen kbal...@webkit.org wrote:
 
  For me optional seems very misleading and generally different prefixes 
  suggests that those objects are not the same.
  Maybe IfExists does not sound nicely but at least it's clear. I would 
  choose to have a pointer version with IfExists and a reference version 
  which is a noun, like:
 
  StyleResolver* styleResolverIfExists()
 
 I like this more. I like that the use of 'if' in the name alerts me to the 
 fact that the function will return something conditionally.
 
 By contrast, existingFoo only makes sense to me if I'm already aware of the 
 idiom. And although I probably will *become* aware of the idiom it will still 
 nonetheless be one of many idioms that I have to be aware of, so I fear that 
 I'll forget why Foo is qualified with existing. That's why I like 
 fooIfExists - its super explicit about what is going on.
 
 I concur.  Maybe
 StyleResolver* styleResolverIfExists()
 StyleResolver styleResolver()
 ?

I concur with this.

For this entire discussion, this is where I was hoping it was headed.

Thanks,
~Brady

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


Re: [webkit-dev] For your consideration: Naming scheme for fooIfExists/ensureFoo

2013-07-01 Thread Dan Bernstein

On Jul 1, 2013, at 7:31 PM, Brady Eidson beid...@apple.com wrote:

 
 On Jul 1, 2013, at 5:27 PM, Ryosuke Niwa rn...@webkit.org wrote:
 
 On Sun, Jun 30, 2013 at 9:39 PM, Filip Pizlo fpi...@apple.com wrote:
 On Jun 19, 2013, at 9:41 AM, Dan Bernstein m...@apple.com wrote:
 
 
 
  On Jun 19, 2013, at 7:37 PM, Timothy Hatcher timo...@apple.com wrote:
 
  What about?
 
  StyleResolver* existingStyleResolver()
  StyleResolver styleResolver()
 
  I like it.
 
 
  — Timothy Hatcher
 
 
  On Jun 19, 2013, at 11:49 AM, Balazs Kelemen kbal...@webkit.org wrote:
 
  For me optional seems very misleading and generally different prefixes 
  suggests that those objects are not the same.
  Maybe IfExists does not sound nicely but at least it's clear. I would 
  choose to have a pointer version with IfExists and a reference version 
  which is a noun, like:
 
  StyleResolver* styleResolverIfExists()
 
 I like this more. I like that the use of 'if' in the name alerts me to the 
 fact that the function will return something conditionally.
 
 By contrast, existingFoo only makes sense to me if I'm already aware of 
 the idiom. And although I probably will *become* aware of the idiom it will 
 still nonetheless be one of many idioms that I have to be aware of, so I 
 fear that I'll forget why Foo is qualified with existing. That's why I 
 like fooIfExists - its super explicit about what is going on.
 
 I concur.  Maybe
 StyleResolver* styleResolverIfExists()
 StyleResolver styleResolver()
 ?
 
 I concur with this.
 
 For this entire discussion, this is where I was hoping it was headed.

On second and third thought, I do think these are better.___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] For your consideration: Naming scheme for fooIfExists/ensureFoo

2013-06-30 Thread Dan Bernstein


On Jun 19, 2013, at 7:37 PM, Timothy Hatcher timo...@apple.com wrote:

 What about?
 
 StyleResolver* existingStyleResolver()
 StyleResolver styleResolver()

I like it.

 
 — Timothy Hatcher
 
 
 On Jun 19, 2013, at 11:49 AM, Balazs Kelemen kbal...@webkit.org wrote:
 
 For me optional seems very misleading and generally different prefixes 
 suggests that those objects are not the same.
 Maybe IfExists does not sound nicely but at least it's clear. I would choose 
 to have a pointer version with IfExists and a reference version which is a 
 noun, like:
 
 StyleResolver* styleResolverIfExists()
 StyleResolver styleResolver()
 
 Br,
 -Balazs
 
 ___
 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] For your consideration: Naming scheme for fooIfExists/ensureFoo

2013-06-30 Thread Filip Pizlo


Sent from my PDP-11

On Jun 19, 2013, at 9:41 AM, Dan Bernstein m...@apple.com wrote:

 
 
 On Jun 19, 2013, at 7:37 PM, Timothy Hatcher timo...@apple.com wrote:
 
 What about?
 
 StyleResolver* existingStyleResolver()
 StyleResolver styleResolver()
 
 I like it.
 
 
 — Timothy Hatcher
 
 
 On Jun 19, 2013, at 11:49 AM, Balazs Kelemen kbal...@webkit.org wrote:
 
 For me optional seems very misleading and generally different prefixes 
 suggests that those objects are not the same.
 Maybe IfExists does not sound nicely but at least it's clear. I would 
 choose to have a pointer version with IfExists and a reference version 
 which is a noun, like:
 
 StyleResolver* styleResolverIfExists()

I like this more. I like that the use of 'if' in the name alerts me to the fact 
that the function will return something conditionally. 

By contrast, existingFoo only makes sense to me if I'm already aware of the 
idiom. And although I probably will *become* aware of the idiom it will still 
nonetheless be one of many idioms that I have to be aware of, so I fear that 
I'll forget why Foo is qualified with existing. That's why I like 
fooIfExists - its super explicit about what is going on. 

-F

 StyleResolver styleResolver()
 
 Br,
 -Balazs
 
 ___
 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
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] For your consideration: Naming scheme for fooIfExists/ensureFoo

2013-06-21 Thread Maciej Stachowiak

On Jun 19, 2013, at 4:22 PM, Elliott Sprehn espr...@chromium.org wrote:

 
 On Wed, Jun 19, 2013 at 9:46 AM, Andreas Kling akl...@apple.com wrote:
 On Jun 19, 2013, at 6:37 PM, Timothy Hatcher timo...@apple.com wrote:
 
 What about?
 
 StyleResolver* existingStyleResolver()
 StyleResolver styleResolver()
 
 
 This doesn't make sense since calling styleResolver() again won't create a 
 new one so it's also existing style resolver.
 
 I rather like the foo() and ensureFoo() methods. foo() is just a plain getter 
 like any other method, the class may start with:
 
 setFoo(Foo*);
 Foo* foo();
 
 later we want to also allow optionally created when needed so we add:
 
 Foo* ensureFoo();
 
 The current naming and methodology makes a lot of sense. fooIfExists() always 
 bugs me because there's no reason to decorate a getter that is just a plain 
 getter. Adding an ensureFoo() method shouldn't make me rename the existing 
 foo() method to fooIfExists().

Personally, I think lazy creation is a more natural getter semantic than 
getting only if something already happened to trigger creation. So I think 
fooIfExists/foo is a more natural pair than foo/ensureFoo. But I agree with 
others on the thread that optionalFoo or existingFoo might be better names, 
particularly given std::optional.

(It might even make sense to have optionalFoo methods return a std::optional 
holding a reference instead of a raw pointer, if it would not be too much of a 
perf hit.)

Regards,
Maciej

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


Re: [webkit-dev] For your consideration: Naming scheme for fooIfExists/ensureFoo

2013-06-19 Thread Gavin Barraclough
I find ‘requireStyleResolver()’ a little confusing.  My first expectation is 
often that a method is an imperative command on the receiver, so I first read 
'requireStyleResolver()’ as mandating that the document now requires a 
StyleResolver, rather than referring to the need of the caller.

In a previous codebase I’ve worked on we used ‘acquire’ as a term of art for 
lazy creation (as a synonym for get, with an inherent connotation that doing so 
may nontrivial).

StyleResolver acquireStyleResolver();

cheers,
G.

On Jun 18, 2013, at 7:11 PM, Darin Adler da...@apple.com wrote:

 On Jun 18, 2013, at 7:05 PM, Ryosuke Niwa rn...@webkit.org wrote:
 
 Why don't we call it requireStyleResolver() instead?
 
 I’m warming to this idea. Maybe we can use “require” as a term of art, 
 analogous to the way we use “create”, to mean “create if not already created”.
 
 -- Darin
 ___
 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] For your consideration: Naming scheme for fooIfExists/ensureFoo

2013-06-19 Thread Maciej Stachowiak

On Jun 18, 2013, at 10:16 PM, Ryosuke Niwa rn...@webkit.org wrote:

 On Tue, Jun 18, 2013 at 7:20 PM, Simon Fraser simon.fra...@apple.com wrote:
 On Jun 18, 2013, at 7:11 PM, Darin Adler da...@apple.com wrote:
 
 On Jun 18, 2013, at 7:05 PM, Ryosuke Niwa rn...@webkit.org wrote:
 
 Why don't we call it requireStyleResolver() instead?
 
 I’m warming to this idea. Maybe we can use “require” as a term of art, 
 analogous to the way we use “create”, to mean “create if not already 
 created”.
 
 Since the fact that it returns a reference implies that it must create 
 something if necessary, the “required” part of the name seems redundant. Why 
 not just
  StyleResolver styleResolver()
 
 requireStyleResolver() sounds like it would return a bool.
 
 True. But it's important to differentiate a simple inline accessor and a 
 lazily-create function because it's very easy to write code like:
 
 if (styleResolver().x())
 styleResolver().y();
 
 and incur two function calls when we could have done
 
 StyleResolver resolver = styleResolver();
 if (resolver.x())
 resolver.y();
 
 instead.
 
 On the other hand, I've started to think that maybe we can simply forbid the 
 former style altogether in the style guide so that we'll never have to think 
 about whether a given function is inline or not.

I don't think possible lazy creation is a good reason to decorate the name. 
Functions should be named for what they do, not their presumed efficiency.

I am also not sure we need a style guideline about putting things into 
variables. If it makes a difference in a hot code path, then sure, but most of 
the time, the more concise code is better.

 - Maciej

 
 - R. Niwa
 
 ___
 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] For your consideration: Naming scheme for fooIfExists/ensureFoo

2013-06-19 Thread Balazs Kelemen
For me optional seems very misleading and generally different prefixes 
suggests that those objects are not the same.
Maybe IfExists does not sound nicely but at least it's clear. I would 
choose to have a pointer version with IfExists and a reference version 
which is a noun, like:


StyleResolver* styleResolverIfExists()
StyleResolver styleResolver()

Br,
-Balazs


On 06/19/2013 10:28 AM, Maciej Stachowiak wrote:


On Jun 18, 2013, at 10:16 PM, Ryosuke Niwa rn...@webkit.org 
mailto:rn...@webkit.org wrote:


On Tue, Jun 18, 2013 at 7:20 PM, Simon Fraser simon.fra...@apple.com 
mailto:simon.fra...@apple.com wrote:


On Jun 18, 2013, at 7:11 PM, Darin Adler da...@apple.com
mailto:da...@apple.com wrote:


On Jun 18, 2013, at 7:05 PM, Ryosuke Niwa rn...@webkit.org
mailto:rn...@webkit.org wrote:


Why don't we call it requireStyleResolver() instead?


I'm warming to this idea. Maybe we can use require as a term
of art, analogous to the way we use create, to mean create if
not already created.


Since the fact that it returns a reference implies that it must
create something if necessary, the required part of the name
seems redundant. Why not just
StyleResolver styleResolver()

requireStyleResolver() sounds like it would return a bool.


True. But it's important to differentiate a simple inline accessor 
and a lazily-create function because it's very easy to write code like:


if (styleResolver().x())
styleResolver().y();

and incur two function calls when we could have done

StyleResolver resolver = styleResolver();
if (resolver.x())
resolver.y();

instead.

On the other hand, I've started to think that maybe we can simply 
forbid the former style altogether in the style guide so that we'll 
never have to think about whether a given function is inline or not.


I don't think possible lazy creation is a good reason to decorate the 
name. Functions should be named for what they do, not their presumed 
efficiency.


I am also not sure we need a style guideline about putting things into 
variables. If it makes a difference in a hot code path, then sure, but 
most of the time, the more concise code is better.


 - Maciej



- R. Niwa

___
webkit-dev mailing list
webkit-dev@lists.webkit.org mailto: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


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


Re: [webkit-dev] For your consideration: Naming scheme for fooIfExists/ensureFoo

2013-06-19 Thread Timothy Hatcher
What about?

StyleResolver* existingStyleResolver()
StyleResolver styleResolver()

— Timothy Hatcher


On Jun 19, 2013, at 11:49 AM, Balazs Kelemen kbal...@webkit.org wrote:

 For me optional seems very misleading and generally different prefixes 
 suggests that those objects are not the same.
 Maybe IfExists does not sound nicely but at least it's clear. I would choose 
 to have a pointer version with IfExists and a reference version which is a 
 noun, like:
 
 StyleResolver* styleResolverIfExists()
 StyleResolver styleResolver()
 
 Br,
 -Balazs

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


Re: [webkit-dev] For your consideration: Naming scheme for fooIfExists/ensureFoo

2013-06-19 Thread Andreas Kling
On Jun 19, 2013, at 6:37 PM, Timothy Hatcher timo...@apple.com wrote:

 What about?
 
 StyleResolver* existingStyleResolver()
 StyleResolver styleResolver()

+1 to these two.

-Kling AKA the guy who named the methods we’re bike shedding about. :|___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] For your consideration: Naming scheme for fooIfExists/ensureFoo

2013-06-19 Thread Geoffrey Garen
On Jun 18, 2013, at 7:03 PM, Emil A Eklund e...@chromium.org wrote:

 +1, much clearer and the pointer vs reference makes it even more so.
 Perhaps enough so that the required prefix could be dropped:
 
 StyleResolver* optionalStyleResolver();
 StyleResolver styleResolver();

I love this!

On Jun 18, 2013, at 10:16 PM, Ryosuke Niwa rn...@webkit.org wrote:

 True. But it's important to differentiate a simple inline accessor and a 
 lazily-create function because it's very easy to write code like:
 
 if (styleResolver().x())
 styleResolver().y();

Like Maciej, I disagree on this point.

If we gave special names to every accessor that was not just a load from a 
field, our code would get bloated and not-fun to read:

globalObject-vmWithMaskAndTwoPointerIndirections();
stringImpl-computeAndStoreHashInLinearTime();
etc.

It’s the programmer’s job to understand the efficiency of the primitives he or 
she uses, and to profile hot code to make sure it’s not needlessly inefficient.

On Jun 18, 2013, at 6:38 PM, Darin Adler da...@apple.com wrote:

 It seems like the C++ community likes the name optional for this concept; 
 isn’t there some kind of std::optional template?

Yes: 

http://en.cppreference.com/w/cpp/utility/optional

The class template std::optional manages an optional contained value. The value 
may be in either initialized or uninitialized state.

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


Re: [webkit-dev] For your consideration: Naming scheme for fooIfExists/ensureFoo

2013-06-19 Thread Elliott Sprehn
On Wed, Jun 19, 2013 at 9:46 AM, Andreas Kling akl...@apple.com wrote:

 On Jun 19, 2013, at 6:37 PM, Timothy Hatcher timo...@apple.com wrote:

 What about?

 StyleResolver* existingStyleResolver()
 StyleResolver styleResolver()



This doesn't make sense since calling styleResolver() again won't create a
new one so it's also existing style resolver.

I rather like the foo() and ensureFoo() methods. foo() is just a plain
getter like any other method, the class may start with:

setFoo(Foo*);
Foo* foo();

later we want to also allow optionally created when needed so we add:

Foo* ensureFoo();

The current naming and methodology makes a lot of sense. fooIfExists()
always bugs me because there's no reason to decorate a getter that is just
a plain getter. Adding an ensureFoo() method shouldn't make me rename the
existing foo() method to fooIfExists().

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


[webkit-dev] For your consideration: Naming scheme for fooIfExists/ensureFoo

2013-06-18 Thread Darin Adler
Lets bike shed!

For some time, functions with names like fooIfExists and ensureFoo have been 
bothering me. I find both names kind of opaque and unpleasant. Here’s an 
example:

StyleResolver* styleResolverIfExists();
StyleResolver* ensureStyleResolver()

What do you think of these names instead?

StyleResolver* optionalStyleResolver();
StyleResolver requiredStyleResolver();

I like them better. Note also that I think the requiredStyleResolver function 
should return a reference so nobody is tempted to do null checks. It seems like 
the C++ community likes the name optional for this concept; isn’t there some 
kind of std::optional template?

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


Re: [webkit-dev] For your consideration: Naming scheme for fooIfExists/ensureFoo

2013-06-18 Thread Emil A Eklund
On Tue, Jun 18, 2013 at 6:38 PM, Darin Adler da...@apple.com wrote:
 What do you think of these names instead?

 StyleResolver* optionalStyleResolver();
 StyleResolver requiredStyleResolver();

+1, much clearer and the pointer vs reference makes it even more so.
Perhaps enough so that the required prefix could be dropped:

StyleResolver* optionalStyleResolver();
StyleResolver styleResolver();
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] For your consideration: Naming scheme for fooIfExists/ensureFoo

2013-06-18 Thread Ryosuke Niwa
On Tue, Jun 18, 2013 at 6:38 PM, Darin Adler da...@apple.com wrote:

 Lets bike shed!

 For some time, functions with names like fooIfExists and ensureFoo have
 been bothering me. I find both names kind of opaque and unpleasant. Here’s
 an example:

 StyleResolver* styleResolverIfExists();
 StyleResolver* ensureStyleResolver()

 What do you think of these names instead?

 StyleResolver* optionalStyleResolver();
 StyleResolver requiredStyleResolver();


requiredStyleResolver sounds as if it's a special (required) type of a
style resolver as opposed to the caller requiring it.  Why don't we call it
requireStyleResolver() instead?

Note also that I think the requiredStyleResolver function should return a
 reference so nobody is tempted to do null checks.


Sounds like a great idea.

On Tue, Jun 18, 2013 at 7:03 PM, Emil A Eklund e...@chromium.org wrote:

 On Tue, Jun 18, 2013 at 6:38 PM, Darin Adler da...@apple.com wrote:
  What do you think of these names instead?
 
  StyleResolver* optionalStyleResolver();
  StyleResolver requiredStyleResolver();

 +1, much clearer and the pointer vs reference makes it even more so.
 Perhaps enough so that the required prefix could be dropped:

 StyleResolver* optionalStyleResolver();
 StyleResolver styleResolver();


I think it's important to communicate the runtime cost of ensuring the
existence of the object.

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


Re: [webkit-dev] For your consideration: Naming scheme for fooIfExists/ensureFoo

2013-06-18 Thread Darin Adler
On Jun 18, 2013, at 7:05 PM, Ryosuke Niwa rn...@webkit.org wrote:

 requiredStyleResolver sounds as if it's a special (required) type of a style 
 resolver as opposed to the caller requiring it.

That is true.

 Why don't we call it requireStyleResolver() instead?

Functions with return values don’t read well when they are verb phrases instead 
of noun phrases. It doesn’t make grammatical sense to perform an operation on a 
verb. Other ideas, maybe not so great:

StyleResolver mandatoryStyleResolver();
StyleResolver neededStyleResolver();

Not loving any of these yet, I guess. They seem to have the same problem as 
requiredStyleResolver.

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


Re: [webkit-dev] For your consideration: Naming scheme for fooIfExists/ensureFoo

2013-06-18 Thread Darin Adler
On Jun 18, 2013, at 7:05 PM, Ryosuke Niwa rn...@webkit.org wrote:

 Why don't we call it requireStyleResolver() instead?

I’m warming to this idea. Maybe we can use “require” as a term of art, 
analogous to the way we use “create”, to mean “create if not already created”.

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


Re: [webkit-dev] For your consideration: Naming scheme for fooIfExists/ensureFoo

2013-06-18 Thread Simon Fraser
On Jun 18, 2013, at 7:11 PM, Darin Adler da...@apple.com wrote:

 On Jun 18, 2013, at 7:05 PM, Ryosuke Niwa rn...@webkit.org wrote:
 
 Why don't we call it requireStyleResolver() instead?
 
 I’m warming to this idea. Maybe we can use “require” as a term of art, 
 analogous to the way we use “create”, to mean “create if not already created”.

Since the fact that it returns a reference implies that it must create 
something if necessary, the “required” part of the name seems redundant. Why 
not just
StyleResolver styleResolver()

requireStyleResolver() sounds like it would return a bool.

Simon

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


Re: [webkit-dev] For your consideration: Naming scheme for fooIfExists/ensureFoo

2013-06-18 Thread Maciej Stachowiak
If the semantic is essentially that of a getter that just happens to lazily 
create what it gets on demand, then I don't think require or required is 
needed. It can just be named as a getter. If the side effect is very important 
and especially if clients ever call the function only for its side effect, then 
a verb phrase would be better. I am not sure which applies in this case.

 - Maciej



On Jun 18, 2013, at 7:20 PM, Simon Fraser simon.fra...@apple.com wrote:

 On Jun 18, 2013, at 7:11 PM, Darin Adler da...@apple.com wrote:
 
 On Jun 18, 2013, at 7:05 PM, Ryosuke Niwa rn...@webkit.org wrote:
 
 Why don't we call it requireStyleResolver() instead?
 
 I’m warming to this idea. Maybe we can use “require” as a term of art, 
 analogous to the way we use “create”, to mean “create if not already 
 created”.
 
 Since the fact that it returns a reference implies that it must create 
 something if necessary, the “required” part of the name seems redundant. Why 
 not just
   StyleResolver styleResolver()
 
 requireStyleResolver() sounds like it would return a bool.
 
 Simon
 
 ___
 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] For your consideration: Naming scheme for fooIfExists/ensureFoo

2013-06-18 Thread Ryosuke Niwa
On Tue, Jun 18, 2013 at 7:20 PM, Simon Fraser simon.fra...@apple.comwrote:

 On Jun 18, 2013, at 7:11 PM, Darin Adler da...@apple.com wrote:

 On Jun 18, 2013, at 7:05 PM, Ryosuke Niwa rn...@webkit.org wrote:

 Why don't we call it requireStyleResolver() instead?


 I’m warming to this idea. Maybe we can use “require” as a term of art,
 analogous to the way we use “create”, to mean “create if not already
 created”.


 Since the fact that it returns a reference implies that it must create
 something if necessary, the “required” part of the name seems redundant.
 Why not just
 StyleResolver styleResolver()

 requireStyleResolver() sounds like it would return a bool.


True. But it's important to differentiate a simple inline accessor and a
lazily-create function because it's very easy to write code like:

if (styleResolver().x())
styleResolver().y();

and incur two function calls when we could have done

StyleResolver resolver = styleResolver();
if (resolver.x())
resolver.y();

instead.

On the other hand, I've started to think that maybe we can simply forbid
the former style altogether in the style guide so that we'll never have to
think about whether a given function is inline or not.

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