Skip to content
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

Control flow keywords should begin their line #785

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Drenmi
Copy link
Contributor

@Drenmi Drenmi commented Dec 31, 2019

Rationale

Breaking control flow is almost certainly the most significant thing that is going on in that line of code, and so it deserves the most prominent spot, at the beginning of the line.

Hiding away control flow in the middle of a long line makes it unnecessarily hard to parse, and Ruby gives us a slew of forms in which we can rewrite the line with the change in flow emphasized.

@Drenmi Drenmi changed the title Add rule stating control flow keywords should begin their line Control flow keywords should begin their line Dec 31, 2019
@koic
Copy link
Member

koic commented Dec 31, 2019

The following idioms exist for return and raise in Ruby.

  • do_something and return
  • do_something || raise

For example, Rails framework will show users an error message using this idiom.

"redirect_to(...) and return\"

https://github.com/rails/rails/blob/v6.0.2.1/actionpack/lib/abstract_controller/rendering.rb#L10

So I think these cases need to be accepted.

@bbatsov
Copy link
Collaborator

bbatsov commented Feb 27, 2020

Agreed. I guess we can word this better and provide more examples.

@Drenmi
Copy link
Contributor Author

Drenmi commented Mar 13, 2020

Hm. I would add a note about its usage in Rails, but I would be hesitant to recommend it in the style guide. It being used in the wild I think should not be seen as an indication that it's good. 😬

I would further argue and return is a Rails idiom, not a Ruby one. (At least I've only ever seen it in Rails codebases.) It is generally also not needed. I've worked with Rails full time for the past 6 years, and never written and return. 🙂

Will update the PR with the rationale for this rule, as well as notes on the usage of and return.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants