No returns

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.