Code Style: Opinion on returning void

# Daniel Bates (14 days ago)

Something bothers me about code like:

void f(); void g() { if (...) return f(); return f(); }

I prefer:

void g() { if (...) { f(); return } f(); }

Does it bother you? For the former? For the latter? Update our style guide?

Opinions, please.

Dan

Contact us to advertise here
# Tim Horton (14 days ago)

On Feb 7, 2019, at 12:47 PM, Daniel Bates <dbates at webkit.org> wrote:

Hi all,

Something bothers me about code like:

void f(); void g() { if (...) return f(); return f(); }

⸘do people do this‽

I prefer:

void g() { if (...) { f(); return } f(); }

Does it bother you? For the former? For the latter? Update our style guide?

+1 to a style guide update

# Ryosuke Niwa (14 days ago)

On Thu, Feb 7, 2019 at 12:53 PM Tim Horton <timothy_horton at apple.com> wrote:

On Feb 7, 2019, at 12:47 PM, Daniel Bates <dbates at webkit.org> wrote:

Hi all,

Something bothers me about code like:

void f(); void g() { if (...) return f(); return f(); }

⸘do people do this‽

I much prefer doing this in my own code but stay away from it in WebKit because we tend to have a separate return.

I prefer: >

void g() { if (...) { f(); return } f(); }

Does it bother you? For the former? For the latter? Update our style guide?

+1 to a style guide update

Yeah, we might as well as codify it in the style guideline for clarity.

  • R. Niwa
# Saam Barati (14 days ago)

On Feb 7, 2019, at 12:53 PM, Tim Horton <timothy_horton at apple.com> wrote:

On Feb 7, 2019, at 12:47 PM, Daniel Bates <dbates at webkit.org> wrote:

Hi all,

Something bothers me about code like:

void f(); void g() { if (...) return f(); return f(); }

⸘do people do this‽

I've definitely done this in JSC. I don't think it's super common, but I've also seen code in JSC not written by me that also does this.

# Geoffrey Garen (14 days ago)

FWIW, I’ve always felt conflicted about this case.

I very much prefer early return to if/else chains.

However, “return f()” when f returns void is a bit mind bending.

So, I can’t use my preferred style in functions that return void. Boo.

Geoff

# Alex Christensen (14 days ago)

If you search for “return completionHandler” in WebKit you will find over a hundred instances. Most if not all of them return void. It means call the completion handler and return. I probably wrote most of them and obviously think it’s a fabulous idiom.

# Zalan Bujtas (14 days ago)

I use this idiom too in the layout code. I guess I just prefer a more compact code. (I don't feel too strongly about it though)

Alan.

On Thu, Feb 7, 2019 at 7:31 PM Alex Christensen <achristensen at apple.com>

wrote:

# Chris Dumez (14 days ago)

Same here, I used it in PSON code with completion handlers. I liked the more concise code but I also do not feel strongly about it.

The extra return line often would have meant adding curly brackets for if statements leading to early returns.

Chris Dumez

# Keith Miller (8 days ago)

I’ve definitely done this in JSC before. As with everyone else, I don’t feel particularly strongly about it.

On Feb 7, 2019, at 8:45 PM, Chris Dumez <cdumez at apple.com> wrote:

Same here, I used it in PSON code with completion handlers. I liked the more concise code but I also do not feel strongly about it.

The extra return line often would have meant adding curly brackets for if statements leading to early returns.

IMO, this is why I think all if/elses should have curlies that way I don’t have to think about whether or not I need them, but ¯_(ツ)_/¯. It would also make refactoring code much easier.

# Daniel Bates (2 days ago)

On Feb 7, 2019, at 12:47 PM, Daniel Bates <dbates at webkit.org> wrote:

Hi all,

Something bothers me about code like:

void f(); void g() { if (...) return f(); return f(); }

I prefer:

void g() { if (...) { f(); return } f(); }

Based on the responses it seems there is sufficient leaning to codify the latter style.

Patch posted bugs.webkit.org/show_bug.cgi?id=194848 for

review of language and examples.

Summary of this thread is in that bug, too.

Dan

# Ryosuke Niwa (2 days ago)

On Tue, Feb 19, 2019 at 8:59 PM Daniel Bates <dbates at webkit.org> wrote:

On Feb 7, 2019, at 12:47 PM, Daniel Bates <dbates at webkit.org> wrote:

Hi all,

Something bothers me about code like:

void f(); void g() { if (...) return f(); return f(); }

I prefer:

void g() { if (...) { f(); return } f(); }

Based on the responses it seems there is sufficient leaning to codify the latter style.

I don't think there is a sufficient consensus as far as I can tell. Geoff and Alex both expressed preferences for being able to return void, and Saam pointed out that there is a lot of existing code which does this. Zalan also said he does this in his layout code.

  • R. Niwa
# Daniel Bates (2 days ago)

On Feb 19, 2019, at 9:42 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:

On Tue, Feb 19, 2019 at 8:59 PM Daniel Bates <dbates at webkit.org> wrote:

On Feb 7, 2019, at 12:47 PM, Daniel Bates <dbates at webkit.org> wrote:

Hi all,

Something bothers me about code like:

void f(); void g() { if (...) return f(); return f(); }

I prefer:

void g() { if (...) { f(); return } f(); }

Based on the responses it seems there is sufficient leaning to codify the latter style.

I don't think there is a sufficient consensus as far as I can tell. Geoff

I didn't get this from Geoff's remark. Geoff wrote:

“return f()” when f returns void is a bit mind bending.

Don't want to put words in Geoff's mouth. So, Geoff can you please confirm: for the former style, for the latter style, no strong opinion.

and Alex both expressed preferences for being able to return void,

I got this from Alex's message

and Saam pointed out that there is a lot of existing code which does this.

I did not get this. He wrote emphasis mine:

I've definitely done this in JSC. I don't think it's super common, but I've also seen code in JSC not written by me that also does this.

Zalan also said he does this in his layout code.

I did not get this, quoting, emphasis mine:

I use this idiom too in the layout code. I guess I just prefer a more compact code. (I don't feel too strongly about it though)

By the way, you even acknowledged that "WebKit ... tend[s] to have a separate return.". So, I inferred you were okay with it. But from this email I am no longer sure what your position is. Please state it clearly.

  • R. Niwa
# Chris Dumez (a day ago)

I also prefer allowed returning void.

Chris Dumez

# Saam Barati (a day ago)

I prefer it as well.

# Daniel Bates (a day ago)

Okay, you’ve changed your mind from your earlier email of not having a strong opinion. Would have been good to know from the get-go. Better late than never knowing :/

Dan

On Feb 20, 2019, at 6:58 AM, Chris Dumez <cdumez at apple.com> wrote:

I also prefer allowed returning void.

Chris Dumez

On Feb 19, 2019, at 10:35 PM, Daniel Bates <dbates at webkit.org> wrote:

On Feb 19, 2019, at 9:42 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:

On Tue, Feb 19, 2019 at 8:59 PM Daniel Bates <dbates at webkit.org> wrote:

On Feb 7, 2019, at 12:47 PM, Daniel Bates <dbates at webkit.org> wrote:

Hi all,

Something bothers me about code like:

void f(); void g() { if (...) return f(); return f(); }

I prefer:

void g() { if (...) { f(); return } f(); }

Based on the responses it seems there is sufficient leaning to codify the latter style.

I don't think there is a sufficient consensus as far as I can tell. Geoff

I didn't get this from Geoff's remark. Geoff wrote:

“return f()” when f returns void is a bit mind bending.

Don't want to put words in Geoff's mouth. So, Geoff can you please confirm: for the former style, for the latter style, no strong opinion.

and Alex both expressed preferences for being able to return void,

I got this from Alex's message

and Saam pointed out that there is a lot of existing code which does this.

I did not get this. He wrote emphasis mine:

I've definitely done this in JSC. I don't think it's super common, but I've also seen code in JSC not written by me that also does this.

Zalan also said he does this in his layout code.

I did not get this, quoting, emphasis mine:

I use this idiom too in the layout code. I guess I just prefer a more compact code. (I don't feel too strongly about it though)

By the way, you even acknowledged that "WebKit ... tend[s] to have a separate return.". So, I inferred you were okay with it. But from this email I am no longer sure what your position is. Please state it clearly.

  • R. Niwa
# Daniel Bates (a day ago)

Thanks for the opinion!

Dan

On Feb 20, 2019, at 7:26 AM, Saam Barati <sbarati at apple.com> wrote:

I prefer it as well.

  • Saam

On Feb 20, 2019, at 6:58 AM, Chris Dumez <cdumez at apple.com> wrote:

I also prefer allowed returning void.

Chris Dumez

On Feb 19, 2019, at 10:35 PM, Daniel Bates <dbates at webkit.org> wrote:

On Feb 19, 2019, at 9:42 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:

On Tue, Feb 19, 2019 at 8:59 PM Daniel Bates <dbates at webkit.org> wrote:

On Feb 7, 2019, at 12:47 PM, Daniel Bates <dbates at webkit.org> wrote:

Hi all,

Something bothers me about code like:

void f(); void g() { if (...) return f(); return f(); }

I prefer:

void g() { if (...) { f(); return } f(); }

Based on the responses it seems there is sufficient leaning to codify the latter style.

I don't think there is a sufficient consensus as far as I can tell. Geoff

I didn't get this from Geoff's remark. Geoff wrote:

“return f()” when f returns void is a bit mind bending.

Don't want to put words in Geoff's mouth. So, Geoff can you please confirm: for the former style, for the latter style, no strong opinion.

and Alex both expressed preferences for being able to return void,

I got this from Alex's message

and Saam pointed out that there is a lot of existing code which does this.

I did not get this. He wrote emphasis mine:

I've definitely done this in JSC. I don't think it's super common, but I've also seen code in JSC not written by me that also does this.

Zalan also said he does this in his layout code.

I did not get this, quoting, emphasis mine:

I use this idiom too in the layout code. I guess I just prefer a more compact code. (I don't feel too strongly about it though)

By the way, you even acknowledged that "WebKit ... tend[s] to have a separate return.". So, I inferred you were okay with it. But from this email I am no longer sure what your position is. Please state it clearly.

  • R. Niwa
# Chris Dumez (a day ago)

On Feb 20, 2019, at 7:48 AM, Daniel Bates <dbates at webkit.org> wrote:

Okay, you’ve changed your mind from your earlier email of not having a strong opinion. Would have been good to know from the get-go. Better late than never knowing :/

I did not change my mind. I said I was using this pattern in my code. So did other people. If we use it in our code, it is because we prefer it. What I meant to say is that if a majority of people felt strongly that we should not allow this pattern, then I would not stand in the way.

I don’t think this mail thread showed that people strongly feel that we should not allow this pattern. Therefore, I was also surprised by your email saying that we’d reached a consensus.

# Simon Fraser (a day ago)

I find it mind bending. It makes me wonder if the author made a coding error.

Simon

# Mark Lam (a day ago)

I also prefer it, and I think some coding patterns may require it e.g. in templates where sometimes we want to specialize into a void function, and other times into a function that returns a value. However, this is rarely needed in practice. Without being able to return void, writing such a template will be a pain if not impossible.

Mark

# Said Abou-Hallawa (a day ago)

On Feb 20, 2019, at 10:35 AM, Mark Lam <mark.lam at apple.com> wrote:

I also prefer it, and I think some coding patterns may require it e.g. in templates where sometimes we want to specialize into a void function, and other times into a function that returns a value. However, this is rarely needed in practice. Without being able to return void, writing such a template will be a pain if not impossible.

But the template and the macro cases are different. The void return in this case is implicit and you do not recognize it unless you see the caller.

template<typename Functor>

auto caller(const Functor& functor) { return functor(); }

Nothing in the return statement above says it will return void. But if you pass it a pointer to a returning void function it will. So outside the scope of macros and templates, I think returning void is not nor really needed.

# Michael Catanzaro (a day ago)

On Wed, Feb 20, 2019 at 12:33 PM, Simon Fraser <simon.fraser at apple.com>

wrote:

I find it mind bending. It makes me wonder if the author made a coding error.

Yeah me too. It does seem to work nicely in Alex's CompletionHandler example, but still, I'd just add braces and return on an extra line if I was writing it.

Anyway, it seems we don't have any consensus here, so no need to create a rule.

Michael

# Maciej Stachowiak (a day ago)

It seems like return foo(); where foo() is a void function can always be replaced with foo(); return; for greater clarity at the cost of one extra line break. For people who prefer the one-line style, can you say why you don’t like the other way?

# Chris Dumez (a day ago)

On Feb 20, 2019, at 1:14 PM, Maciej Stachowiak <mjs at apple.com> wrote:

It seems like return foo(); where foo() is a void function can always be replaced with foo(); return; for greater clarity at the cost of one extra line break. For people who prefer the one-line style, can you say why you don’t like the other way?

We do not allow more than one statement per line so it would be: foo(); return;

Also, since we favor early returns in WebKit, things like: If (!nok) return completionHandler(Failed);

Would become: If (!nok) { completionHandler(Failed); return; }

It is not a big deal but I personally prefer the most concise version. Especially, it is not uncommon to have multiple early returns. I think more concise is better and I personally do not see a readability issue here. It does not really matter what the completion handler is returning.

# Alex Christensen (a day ago)

I like it mostly for its brevity, but I also think it would be strange that changing a return type from bool to void or vice versa would require touching all its call sites.

# Darin Adler (5 hours ago)

I tried not to weigh in on this, but my view on the materials we should use to build the bike shed must be shared!

Generally it seems neat to be able to make the code slightly more tight and terse by merging the function call and the return into a single statement.

Other than not being accustomed to it, I have noticed three small things I don’t like about using “return” with a call to a void-returning function.

  • You can’t use this idiom when you want to ignore the result of a function, only if the function actually returns void. Often functions are designed with ignorable and often ignored return values. For example, it’s common to call something that might fail and have a good reason to ignore whether it failed or not. The idiom we are discussing requires treating those functions differently from ones that return void. If you refactor so that a function now returns an ignorable return value you need to visit all the call sites using return and change them into the two line form.

  • It works for return, but not for break. I’ve seen at least one case where someone filled a switch statement with return statements instead of break statements so they could use this more-terse form. Now if we want to add one line of code after that switch, we need to convert all those cases into multi-line statements with break.

  • Unless it’s mandatory, it’s a case where two programmers can make different style choices. If both forms are acceptable, then it introduces a little bit of disorder into our code.

One thing I like about it is that since “pass along the return value from this inner function to this outer function” can be written this way, the code can then survive refactorings where both the inner and outer functions might gain or lose the return value.

— Darin

Want more features?

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