Chris Wanstrath makes a good point about an ugly way to initialize a variable, but I don't agree that an explicit return is the best style to use.
The original ugly:
def logger
unless @logger
@logger = Logger.new(STDOUT)
@logger.level = Logger::WARN
end
@logger
end
Chris' improvement:
def logger
return @logger if defined? @logger
@logger = Logger.new(STDOUT)
@logger.level = Logger::WARN
@logger
end
I prefer a functional style:
def logger
@logger || begin
@logger = Logger.new(STDOUT)
@logger.level = Logger::WARN
@logger
end
end
This is actually slightly awkward. I'd usually do @logger ||=, but that's hard to do gracefully since here you have to combine a few statements to create the new logger. A better constructor or a #level method that returned self would be so much nicer. A small refactoring seems a bit more readable.
def logger
@logger ||= default_logger
end
def default_logger
logger = Logger.new(STDOUT)
logger.level = Logger::WARN
logger
end
And if you really like functional style and are a fan of the K combinator:
def logger
@logger ||= Logger.new(STDOUT).tap {|l| l.level = Logger::WARN}
end
I am almost always opposed to using an explicit return in Ruby code. Whenever I see one I cringe. You can sometimes make a case for one as a top guard in a medium-sized method, but here's where I invoke the slippery-slope argument. It's too easy for those things to snowball into a handful of return statements, and then someday you'll have to wonder if they have to be in that order or not. And when your method gets long enough, you can easily miss that there was a return at the top and mess up a refactoring; using an if/then/else makes it clearer and harder to miss.
By the way, using if defined? @logger the way Chris does is not the same thing as if @logger.
>> defined?(@logger)
=> nil
>> @logger
=> nil
>> @logger = nil
=> nil
>> defined?(@logger)
=> "instance-variable"
If you're trying to make sure @logger is not nil, defined? is unlikely to be the way you want to test the variable was initialized. Be careful out there, kids...
UPDATE: Oh awesome, it turns out that Logger.new takes a 2nd arg, the level. So this could all be done with:
def logger
@logger ||= Logger.new(STDOUT, Logger::WARN)
end
Just pretend this whole thing was about some other class where this actually mattered.
