best practice for return values from ruby methods

I find myself doing the following a lot to define return values from ruby methods:

def foo
  val = (some expression)
  va开发者_Go百科l
end

This always seems a bit contrived. What's the best practice here?

---

It is unnecessary to save it to a variable unless (some expression) is heavy and will be called multiple times. In that case you might want to cache it.

I would go with either:

def foo
  (some expression)
end

or for caching:

def foo
  @val ||= (some expression)
end

---

Note that as of Ruby 1.9 you can use Object#tap to save a value for the return at the end if you need to do something else with the value before returning it:

def foo
  (some expression).tap do |val|
    # use val here
  end
  # The return value of the tap is _val_
  # and hence the return value of your method
end

---

As long as your last expression evaluates to the desired one you want to return, you're safe.

def foo
  val = (some expression)
end

is identical to the one in the question as it evaluates to (some expression), just like val would.

---

I personally like using return to explicitly call out what is being returned. It's extra code that Ruby doesn't require you to use, but it helps me with readability. It also allows you to have multiple exit points in your method since execution of your method will stop as soon as return is called.

This really isn't much different from the example you gave in your original question.

def foo
  val = (some expression)
  val
end

could look like

def foo
  return (some expression)
end

---

I sometimes do what you have in your question.

Some cases where I do it are:

1. When I'm doing imperitive programming (foo = Foo.new; foo.modify_state; foo)
2. If I want to validate an object before returning, but as Phrogz mentioned, Object#tap may help here (foo = new_foo; raise if foo.empty?; foo)
3. When I want to make it clear that I'm returning a variable, rather than doing more stuff (do_this; do_that; do_other_thing; result #done!)

It may indicate code smells though, such as in case 1.

---

Temporary variables are evil because they increase connascence.

http://www.mgroves.com/what-is-connascence

ReplaceTempWithQuery is a refactoring I use a lot:

def discount_price
  base_price = quantity * item_price
  if (base_price > 1000)
    base_price * 0.95
  else
    base_price * 0.98
  end
end

Code after refactoring:

def discount_price
  if (base_price > 1000)
    base_price * 0.98
  else
    base_price * 0.98
  end
end

def base_price
  quantity * item_price
end

http://www.refactoring.com/catalog/replaceTempWithQuery.html