Reputation: 13424
I find myself doing the following a lot to define return values from ruby methods:
def foo
val = (some expression)
val
end
This always seems a bit contrived. What's the best practice here?
Upvotes: 4
Views: 2224
Reputation: 303490
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
Upvotes: 9
Reputation: 81651
I sometimes do what you have in your question.
Some cases where I do it are:
foo = Foo.new; foo.modify_state; foo
)Object#tap
may help here (foo = new_foo; raise if foo.empty?; foo
)do_this; do_that; do_other_thing; result #done!
)It may indicate code smells though, such as in case 1.
Upvotes: 1
Reputation: 970
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
Upvotes: 0
Reputation: 560
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
Upvotes: 1
Reputation: 21180
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
Upvotes: 11
Reputation: 4749
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.
Upvotes: 1