Best practices in methods

Hello,

Consider this code, it gets a mention and an array of tags.
It finds the mentions and add the tags.
But in my methods I keep using return if foo.nil?
or execute a line of code if a line is not nil.
Is there a better way of doing this kind of thing?
Thks

def add_tags_to_mention mention_id, *tags
mention = Mention.find mention_id
return if mention.nil?
tags.each do |tag_id|
tag = Tag.find tag_id
mention.tags.push(tag.to_mongo) if tag
end
end

On Tue, Nov 13, 2012 at 6:22 AM, rodrigo coutinho
[email protected] wrote:

tags.each do |tag_id|
tag = Tag.find tag_id
mention.tags.push(tag.to_mongo) if tag
end
end

Personally I would handle that a bit differently in that I would have
the model handle building it’s own tags and I would inject an array []
and then delete_if value blank? but to address your concern directly:

I would never return nil unless I have to. I know quite a few Ruby
programmers do not care about that situation but I always return
false, true or the value of the value I processed… there are a lot
of cases where I will also to opt to just raise but that really
depends… in your situation I would have built it on the model as
Mention.build_and_save_tags and then had it raise if it could not find
the mention because to me that makes the most sense. What I am saying
is to me return false if mention.blank? makes more sense to me. I
could go on a long explanation a mile long explaining the difference
between situations and why I chose my flow to work those ways but I’ll
just leave it at I would prefer false before nil.

Returning nil is a little bit ambiguous. Did the method finished
correctly or a condition wasn’t met?
I always try to return something or throw an exception when needed.

I don’t know if there’s a “best practice” for that.

On Tue 13 Nov 2012 10:24:00 AM ART, Jordon B. wrote:

mention = Mention.find mention_id

just leave it at I would prefer false before nil.

Mis mejores deseos,
Best wishes,
Meilleurs vux,

Juan P.

Alright, based on your opinion of returning nil,I changed the method a
little bit. In fact return false is better.
Thank you.

class Mention

def self.build_and_save_tags mention_id, *tags
mention = Mention.find mention_id
return false if mention.blank?
tags.each do |tag_id|
tag = Tag.find tag_id
if tag
mention.tags.push(tag.to_mongo)
mention.save
end
end
end
end