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

Is return nil a bad style? #557

Closed
BroiSatse opened this issue Apr 8, 2016 · 9 comments
Closed

Is return nil a bad style? #557

BroiSatse opened this issue Apr 8, 2016 · 9 comments
Labels

Comments

@BroiSatse
Copy link

OCD in action: One might argue that return nil is slightly more expressive than return, however I have rather fixed feelings about such a construct. Should it be advised against as a bad style?

def something
  return nil if condition
  do_epic_stuff
end
@equivalent
Copy link
Contributor

...deleted my previous posts as I misunderstood your point

I think most of condition return should be banned in this case, why not to do :

def something
  do_epic_stuff unless condition
end

@BroiSatse
Copy link
Author

@equivalent - do_epic_stuff was meant to be a placeholder for the rest of the code, it is not necessarily one-liner. Returning on first line saves you one level of if nesting. The main question is return nil vs return

@equivalent
Copy link
Contributor

@BroiSatse I stand my ground :)

class Stuff
  def something
    do_epic_stuff unless condition
  end

  private
    def do_epic_stuff
       'really epic script'
    end
end

the style of code I'm arguing has somewhat base in functional programming rules where you want every method to have one outcome and return is considered break of that one functionality

one may argue but this is has nothing to do with OOP, well to be honest it does, as part of single responsibility principle (do_epic_stuff method has one responsibility to execute, while something has responsibility to decide what to do based on condition)

therefore code is easily extendable

class Stuff
  def something
    if  condition
      do_different_stuff
    else
      do_epic_stuff 
    end
  end

  private
    def do_epic_stuff
       'really epic script'
    end

    def do_different_stuff
       'really epic script'
    end
end

@BroiSatse
Copy link
Author

@equivalent
Copy link
Contributor

yep agree once it's there it's there and there is nothing for me to do than obey the rules :) that's one fight I've already lost. Still doh, the argument is still there and I take any chance to bring it up to discussion again :) (e.g. your GH issue)

@BroiSatse
Copy link
Author

Well those are not strictly rules, I agree that there are cases when if is both more expressive and readable. I am very used to do such returns now (it is also advised in Uncle Bob's Clean Code book to reduce nesting level in functions), but obviously when both positive and negative path are quite long splitting it into separate functions is just pretty.

@lamawithonel
Copy link

IMO, assuming you have good reason to use return with a nil value, the implied nil is best. It's not like shell where the return value of previous statements carries over. Ruby's return without a value will always return nil, so adding characters is just needless clutter.

Also, next and break have similar semantics. I can't see any reason why the rules for their use would be any different.

@codebycliff
Copy link

I would argue that the semantics are slightly different. If I see return nil, I expected the caller to care about the return value. If I see return, I assume its a void method that just does work. The result is still the same, but the way the user reads it might be different.

@bbatsov
Copy link
Collaborator

bbatsov commented Jun 12, 2019

@codebycliff I completely agree.

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

No branches or pull requests

5 participants