Code Style: Opinion on returning void
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
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
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.
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
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.
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:
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
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.
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
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
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
I also prefer allowed returning void.
Chris Dumez
I prefer it as well.
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
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
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.
I find it mind bending. It makes me wonder if the author made a coding error.
Simon
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
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.
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
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?
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 withfoo(); 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.
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.
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.
- Go to Source
# 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
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(); } Does it bother you? For the former? For the latter? Update our style guide? Opinions, please. Dan -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20190207/04c1df6e/attachment.html>