For your consideration: Naming scheme for fooIfExists/ensureFoo

# Darin Adler (6 years ago)

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?

Contact us to advertise here
# Emil A Eklund (6 years ago)

On Tue, Jun 18, 2013 at 6:38 PM, Darin Adler <darin at 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();

# Ryosuke Niwa (6 years ago)

On Tue, Jun 18, 2013 at 6:38 PM, Darin Adler <darin at 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 <eae at chromium.org> wrote:

On Tue, Jun 18, 2013 at 6:38 PM, Darin Adler <darin at 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
# Darin Adler (6 years ago)

On Jun 18, 2013, at 7:05 PM, Ryosuke Niwa <rniwa at 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 Adler (6 years ago)

On Jun 18, 2013, at 7:05 PM, Ryosuke Niwa <rniwa at 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”.

# Simon Fraser (6 years ago)

On Jun 18, 2013, at 7:11 PM, Darin Adler <darin at apple.com> wrote:

On Jun 18, 2013, at 7:05 PM, Ryosuke Niwa <rniwa at 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

# Maciej Stachowiak (6 years ago)

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.

# Ryosuke Niwa (6 years ago)

On Tue, Jun 18, 2013 at 7:20 PM, Simon Fraser <simon.fraser at apple.com>wrote:

On Jun 18, 2013, at 7:11 PM, Darin Adler <darin at apple.com> wrote:

On Jun 18, 2013, at 7:05 PM, Ryosuke Niwa <rniwa at 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
# Gavin Barraclough (6 years ago)

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();
# Maciej Stachowiak (6 years ago)

On Jun 18, 2013, at 10:16 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:

On Tue, Jun 18, 2013 at 7:20 PM, Simon Fraser <simon.fraser at apple.com> wrote:

On Jun 18, 2013, at 7:11 PM, Darin Adler <darin at apple.com> wrote:

On Jun 18, 2013, at 7:05 PM, Ryosuke Niwa <rniwa at 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.

# Balazs Kelemen (6 years ago)

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,

# Timothy Hatcher (6 years ago)

What about?

StyleResolver* existingStyleResolver() StyleResolver& styleResolver()

— Timothy Hatcher

# Dan Bernstein (6 years ago)

On Jun 19, 2013, at 7:37 PM, Timothy Hatcher <timothy at apple.com> wrote:

What about?

StyleResolver* existingStyleResolver() StyleResolver& styleResolver()

I like it.

# Andreas Kling (6 years ago)

On Jun 19, 2013, at 6:37 PM, Timothy Hatcher <timothy at 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. :|

# Geoffrey Garen (6 years ago)

On Jun 18, 2013, at 7:03 PM, Emil A Eklund <eae at 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 <rniwa at 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 <darin at 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:

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

# Elliott Sprehn (6 years ago)

On Wed, Jun 19, 2013 at 9:46 AM, Andreas Kling <akling at apple.com> wrote:

On Jun 19, 2013, at 6:37 PM, Timothy Hatcher <timothy at 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().

# Maciej Stachowiak (6 years ago)

On Jun 19, 2013, at 4:22 PM, Elliott Sprehn <esprehn at chromium.org> wrote:

On Wed, Jun 19, 2013 at 9:46 AM, Andreas Kling <akling at apple.com> wrote: On Jun 19, 2013, at 6:37 PM, Timothy Hatcher <timothy at 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.)

# Filip Pizlo (6 years ago)

Sent from my PDP-11

On Jun 19, 2013, at 9:41 AM, Dan Bernstein <mitz at apple.com> wrote:

On Jun 19, 2013, at 7:37 PM, Timothy Hatcher <timothy at apple.com> wrote:

What about?

StyleResolver* existingStyleResolver() StyleResolver& styleResolver()

I like it.

— Timothy Hatcher

On Jun 19, 2013, at 11:49 AM, Balazs Kelemen <kbalazs at 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.

# Ryosuke Niwa (6 years ago)

On Sun, Jun 30, 2013 at 9:39 PM, Filip Pizlo <fpizlo at apple.com> wrote:

On Jun 19, 2013, at 9:41 AM, Dan Bernstein <mitz at apple.com> wrote:

> >

On Jun 19, 2013, at 7:37 PM, Timothy Hatcher <timothy at apple.com> wrote:

What about?

StyleResolver* existingStyleResolver() StyleResolver& styleResolver()

I like it.

>

— Timothy Hatcher

On Jun 19, 2013, at 11:49 AM, Balazs Kelemen <kbalazs at 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
# Brady Eidson (6 years ago)

On Jul 1, 2013, at 5:27 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:

On Sun, Jun 30, 2013 at 9:39 PM, Filip Pizlo <fpizlo at apple.com> wrote: On Jun 19, 2013, at 9:41 AM, Dan Bernstein <mitz at apple.com> wrote:

> >

On Jun 19, 2013, at 7:37 PM, Timothy Hatcher <timothy at apple.com> wrote:

What about?

StyleResolver* existingStyleResolver() StyleResolver& styleResolver()

I like it.

>

— Timothy Hatcher

On Jun 19, 2013, at 11:49 AM, Balazs Kelemen <kbalazs at 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

# Dan Bernstein (6 years ago)

On Jul 1, 2013, at 7:31 PM, Brady Eidson <beidson at apple.com> wrote:

On Jul 1, 2013, at 5:27 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:

On Sun, Jun 30, 2013 at 9:39 PM, Filip Pizlo <fpizlo at apple.com> wrote: On Jun 19, 2013, at 9:41 AM, Dan Bernstein <mitz at apple.com> wrote:

> >

On Jun 19, 2013, at 7:37 PM, Timothy Hatcher <timothy at apple.com> wrote:

What about?

StyleResolver* existingStyleResolver() StyleResolver& styleResolver()

I like it.

>

— Timothy Hatcher

On Jun 19, 2013, at 11:49 AM, Balazs Kelemen <kbalazs at 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.

# Maciej Stachowiak (6 years ago)

On Jul 1, 2013, at 7:31 PM, Brady Eidson <beidson at apple.com> wrote:

On Jul 1, 2013, at 5:27 PM, Ryosuke Niwa <rniwa at 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?

# Bem Jones-Bey (6 years ago)

On Jul 2, 2013, at 17:06, "Maciej Stachowiak" <mjs at apple.com<mailto:mjs at apple.com>> wrote:

On Jul 1, 2013, at 7:31 PM, Brady Eidson <beidson at apple.com<mailto:beidson at apple.com>> wrote:

On Jul 1, 2013, at 5:27 PM, Ryosuke Niwa <rniwa at webkit.org<mailto:rniwa at 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.

# Ryosuke Niwa (2 days ago)

I guess we never remembered to update our style guideline back in 2013.

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

  • R. Niwa
# Ryosuke Niwa (2 days ago)

For those who joined webkit-dev after June 2013, see lists.webkit.org/pipermail/webkit-dev/2013-June/thread.html#25056

Want more features?

Request early access to our private beta of readable email premium.