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

Empty line between methods vs no nested conditionals #710

Open
gutekk opened this issue Apr 10, 2018 · 2 comments
Open

Empty line between methods vs no nested conditionals #710

gutekk opened this issue Apr 10, 2018 · 2 comments

Comments

@gutekk
Copy link

gutekk commented Apr 10, 2018

All is about to use empty line after return or not?

First example from
https://github.com/bbatsov/ruby-style-guide#empty-lines-between-methods

def some_method
  data = initialize(options)

  data.manipulate!

  data.result
end

and second example from here
https://github.com/bbatsov/ruby-style-guide#no-nested-conditionals

# good
def compute_thing(thing)
  return unless thing[:foo]
  update_with_bar(thing[:foo])
  return re_compute(thing) unless thing[:foo][:bar]
  partial_compute(thing)
end

After combining first and second examples this shouldnt look like that?

def compute_thing(thing)
  return unless thing[:foo]

  update_with_bar(thing[:foo])

  return re_compute(thing) unless thing[:foo][:bar]

  partial_compute(thing)
end

Should we use empty line after return?

@moritzschepp
Copy link

Ha, good point!

I feel that "Use empty lines between method definitions and also to break up methods into logical paragraphs internally." (for the first example) leaves some room for interpretation. IMHO its not necessary to break up a really simple method body into logical paragraphs. I consider 3 lines to be simple in most cases.

That said, since the return in the middle of a function is a opaque way of flow control, I like the idea of requiring a newline after it. So for your last example, I'd vote for

def compute_thing(thing)
  return unless thing[:foo]

  update_with_bar(thing[:foo])
  return re_compute(thing) unless thing[:foo][:bar]

  partial_compute(thing)
end

Although I would avoid returns of that kind when they can easily be transformed into an if-else block.

@gareth
Copy link
Contributor

gareth commented Apr 12, 2018

Although I would avoid returns of that kind when they can easily be transformed into an if-else block.

If you do that, you're likely to trigger Style/GuardClause (edit: oh, not if there's content in the else block)

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

No branches or pull requests

3 participants