-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
suggest using explicit return in style guide #29727
Conversation
doc/src/manual/style-guide.md
Outdated
function h!(x) | ||
fill!(x, compute(x)) | ||
modify!(x) | ||
return nothing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe just return
or nothing
? explicitly stating the default is a bit excessive
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed on just return
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a convention that mutating functions should return the input, like push!
?
Explicit `return` on an expression makes it clear that the expression is intended to be returned and | ||
is not only used for its side effect. It also makes it possible to locally see that a certain sub-expression | ||
in a larger expression is returned while for an implicit return value one needs to analyze the whole outer expression. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe also mention that it can help the program to run faster if it was returning an unintentional value (when it only needs to return nothing
)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very good point but I think it is quite separate --- whether to end a function with x
or return x
is just a style choice, but making sure to return nothing
when you don't mean to return a value is more significant.
function h!(x) | ||
fill!(x, compute(x)) | ||
modify!(x) | ||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we find an example that doesn't run counter to other suggestions in this file?
It is typical for such functions to also return the modified array for convenience.
I'm not the biggest fan of the first example either as I try to only use one return point if possible. That's not a documented guideline, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed on the first point.
Regarding one return point, I personally think early return is much better. There is seldom need to do a bunch of cleanup in Julia so any advantage of having only one return point seems much reduced compared to e.g. C. If your comment was just about the specific example, I just took the simplest I could figure out, of course, you would typically just write the whole thing there as f(x) = x > 2 ? "a" : "b"
. Can maybe come up with something better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I love early returns (guard clauses) at the top of a function, too, but I'll write those as short-circuits. I don't love returns tucked away in the complicated portion of an algorithm if I can help it. In a short example though it's not clear which is which. What you have is just fine.
Triage: Should be reworded but something along those lines is positive. |
meh |
No description provided.