Bug 194930 – Codify the naming convention for fooIfExists

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Website

[Ryosuke Niwa]

See https://lists.webkit.org/pipermail/webkit-dev/2013-June/025056.html

Better late than never.

[Ryosuke Niwa]

Created attachment 362688 [details]
Updates the style guideline

[Darin Adler]

Comment on attachment 362688 [details]
Updates the style guideline

Looks fine

[Ryosuke Niwa]

I'd wait a day or two to give a chance for people to respond on webkit-dev in the case they don't like this convention since the discussion happened 5 years ago.

[zalan]

Comment on attachment 362688 [details]
Updates the style guideline

View in context: https://bugs.webkit.org/attachment.cgi?id=362688&action=review

> Websites/webkit.org/ChangeLog:8
> +        Updating the coding style guidleline after the discussion following

guideline

[Daniel Bates]

Comment on attachment 362688 [details]
Updates the style guideline

View in context: https://bugs.webkit.org/attachment.cgi?id=362688&action=review

> Websites/webkit.org/code-style.md:686
> +```cpp
> +StyleResolver* styleResolverIfExists();
> +StyleResolver& styleResolver();
> +```

First, if I were to vote today (02/21/2019), I would vote for StyleResolver& styleResolver() and StyleResolver* existingStyleResolver(), which was what Darin suggested when he wrote "I can’t remember what we decided on for our convention. I personally prefix the thing()/existingThing() pairing, but maybe I was outvoted?" in bug #194764, comment 18. Since Darin r+'ed this patch I guess he changed his mind. Funny thing though, if he didn't change his mind then his vote and mine would tip the scale towards:

StyleResolver& styleResolver() and StyleResolver* existingStyleResolver()

Funny, huh?

I don't know how you came to think StyleResolver* styleResolverIfExists() and StyleResolver& styleResolver() are the "right" choice, but Darin r+'ed it. So, I guess he warmed up to styleResolverIfExists() even though he started that webkit-dev thread you mentioned with "Lets bike shed! For some time, functions with names like **fooIfExists** and ensureFoo have been bothering me." I went through every single email in that thread (was a different time back made it clear what their position was with a "+1"). Here's what I found and it disagrees with your choices and I'm only mentioning it for completeness since Darin r+'ed the patch (and again he was the one that was initially unhappy). I did skip a few emails that just gave some other naming suggestions that nobody ever replied to as liking:

Emil A Eklund eae at chromium.org: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025057.html> (BTW she no longer contributes)
    +1 StyleResolver* optionalStyleResolver() and StyleResolver& requiredStyleResolver()
    But suggested StyleResolver* optionalStyleResolver() and StyleResolver& styleResolver() (So to me this implies these are +1 by her as well)

Geoffrey Garen ggaren at apple.com: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025076.html>
     "I love this!" to Emil A Eklund's suggestion of  StyleResolver* optionalStyleResolver() and StyleResolver& styleResolver()

Ryosuke Niwa (You): <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025058.html>
    Suggest requireStyleResolver()

Darin Adler: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025060.html>
    "Tentative +1" = 0.5 to Ryosuke's suggestion of requireStyleResolver(): "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: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025061.html>
    -1 for requireStyleResolver(): "requireStyleResolver() sounds like it would return a bool."

Maciej Stachowiak: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025061.html>
    Sounds like not in favor of requireStyleResolver(). Suggests to drop qualifier.

Balazs Kelemen kbalazs at webkit.org: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025072.html>  (BTW no longer contributes)
    Suggests styleResolverIfExists() and styleResolver()

Timothy Hatcher timothy at apple.com: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025073.html>:
    Suggests existingStyleResolver() and styleResolver().

Dan Bernstein mitz at apple.com: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025073.html>
    "I like it." to Timothy Hatcher's suggestion.

Filip Pizlo fpizlo at apple.com: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025119.html>
    Likes styleResolverIfExists(): "That's why I like "fooIfExists""

Andreas Kling akling at apple.com: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025074.html> (BTW no longer contributes)
    "+1 to these two." to Timothy Hatcher's suggestion of existingStyleResolver() and styleResolver().

Elliott Sprehn esprehn at chromium.org: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025080.html> (BTW no longer contributes)
    -1 to existingStyleResolver() and styleResolver(): "This doesn't make sense since calling styleResolver() again won't create a new one so it's also "existing style resolver"."
    +1 to foo() and ensureFoo()

Maciej Stachowiak mjs at apple.com: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025083.html>
     Disagrees with Elliott's suggestion. Suggests "fooIfExists/foo. But I agree with others on the thread that optionalFoo or existingFoo might be better names, particularly given std::optional"

Gavin Barraclough barraclough at apple.com: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025070.html>
    -1 requireStyleResolver: "I find ‘requireStyleResolver()’ a little confusing."  Suggests acquireStyleResolver().

My composite breakdown of the responses:

Name for "guaranteed to give you a resolver" function: 

StyleResolver& styleResolver() => +8 (Emil A Eklund, Geoffrey Garen, Balazs Kelemen, Timothy Hatcher, Dan Bernstein, Andreas Kling, Elliott Sprehn, Maciej Stachowiak)
StyleResolver& requireStyleResolver() => +1.5 = +1 "Ryosuke Niwa" + 0.5 "Darin Adler"

Name for function that returns, but does not create a resolver:

StyleResolver* styleResolverIfExists() => +3 (Balazs Kelemen, Filip Pizlo, Maciej Stachowiak)
StyleResolver* existingStyleResolver() => +3 (Timothy Hatcher, Dan Bernstein, Andreas Kling)
StyleResolver* optionalStyleResolver() => +2 (Emil A Eklund, Geoffrey Garen)

Adding in my votes and focusing on the winning results:

StyleResolver& styleResolver() => +9 (Emil A Eklund, Geoffrey Garen, Balazs Kelemen, Timothy Hatcher, Dan Bernstein, Andreas Kling, Elliott Sprehn, Maciej Stachowiak, Daniel Bates)
StyleResolver* existingStyleResolver() => +4 (Timothy Hatcher, Dan Bernstein, Andreas Kling, Daniel Bates)

Adding in Darin's votes if he changes his mind back to what he wrote in bug #194764, comment 18, final score is:

StyleResolver& styleResolver() => +10 (Emil A Eklund, Geoffrey Garen, Balazs Kelemen, Timothy Hatcher, Dan Bernstein, Andreas Kling, Elliott Sprehn, Maciej Stachowiak, Daniel Bates, Darin Adler)
StyleResolver* existingStyleResolver() => +5 (Timothy Hatcher, Dan Bernstein, Andreas Kling, Daniel Bates, Darin Adler)

[Daniel Bates]

Darn. I missed some replies that spilled over to July: <https://lists.webkit.org/pipermail/webkit-dev/2013-July/025122.html>

[Daniel Bates]

Ryosuke Niwa (You): <https://lists.webkit.org/pipermail/webkit-dev/2013-July/025122.html>
   +1 Balazs Kelemen's StyleResolver* styleResolverIfExists() and StyleResolver& styleResolver(): "Maybe StyleResolver* styleResolverIfExists() StyleResolver& styleResolver()"

Brady Eidson: <https://lists.webkit.org/pipermail/webkit-dev/2013-July/025123.html>
   +1 StyleResolver* styleResolverIfExists() and StyleResolver& styleResolver()

Dan Bernstein mitz at apple.com: <https://lists.webkit.org/pipermail/webkit-dev/2013-July/025124.html>
   +1 StyleResolver* styleResolverIfExists() and StyleResolver& styleResolver()

Maciej Stachowiak mjs at apple.com: <https://lists.webkit.org/pipermail/webkit-dev/2013-July/025130.html>
   +1 StyleResolver* styleResolverIfExists() and StyleResolver& styleResolver()

Bem Jones-Bey bjonesbe at adobe.com: <https://lists.webkit.org/pipermail/webkit-dev/2013-July/025132.html> (BTW no longer contributes)
   +1 StyleResolver* styleResolverIfExists() and StyleResolver& styleResolver()

So, final tally for styleResolver() and styleResolverIfExists(), not including me or Darin:

StyleResolver& styleResolver() => +10 (Emil A Eklund, Geoffrey Garen, Balazs Kelemen, Timothy Hatcher, Dan Bernstein, Andreas Kling, Elliott Sprehn, Maciej Stachowiak, Brady Eidson, Bem Jones-Bey)

StyleResolver* styleResolverIfExists() => +7 (Balazs Kelemen, Filip Pizlo, Maciej Stachowiak, Ryosuke Niwa, Dan Bernstein, Brady Eidson, Bem Jones-Bey)

So, even if I voted and Darin changed his mind on styleResolverIfExists(), styleResolverIfExists() would win. Okay. I'm fine with that.

[Ryosuke Niwa]

Yeah, it was a pretty lengthy debate and there were many suggestions and up-votes.

It was easier for me to go through because I had an email thread in my email archive. Thanks for summarizing the results though.

[Ryosuke Niwa]

Committed r242273: <https://trac.webkit.org/changeset/242273>

[Radar WebKit Bug Importer]

<rdar://problem/48516506>