Using Dalli to connect to memcached

Question

I always seem to struggle refactoring my code. I can look at any other code and know exactly what is going on, but when it comes to cleaning up my code, I get writers block.

The following code works, but I know if can be done much nicer, and with few lines of code. I just wanted to wrap a memcache client to use one server for testing and another for dev/prod or just fetch the value if it cannot connect to the server.

Any tips on refactoring, etc. are much appreciated.

require 'dalli'
class Cache
    def self.fetch(key, ttl, &block)
        if memcache
            memcache.fetch(key, ttl, &block)
        else
            block.call
        end
    end

    def self.memcache
        begin
        if(ENV['RACK_ENV'] == :production or ENV['RACK_ENV'] == :development)
            @memcache ||= Dalli::Client.new('cache.amazonaws.com:11211')    
        else
            @memcache ||= Dalli::Client.new('localhost:11211')  
        end
        rescue Exception => e
            false
        end
    end
end

Answer 1

This DRY's the code in self.memcache and simply uses a ternary operator for self.fetch.

require 'dalli'
class Cache
  def self.fetch(key, ttl, &block)
    memcache ? memcache.fetch(key, ttl, &block) : block.call
  end

  def self.memcache
    @memcache ||= Dalli::Client.new((ENV['RACK_ENV'] == :production or ENV['RACK_ENV'] == :development) ?
                                    'cache.amazonaws.com:11211' :
                                    'localhost:11211')
  rescue Exception
    false
  end
end

Answer 2

I'd write it like this, if I really want to have fetch as a class method. If that is not mandatory, I would make fetch the only public instance method, and the rest private instance methods.

 require 'dalli'

 class Cache
   def self.fetch(key, ttl, &block)
     memcache ? memcache.fetch(key, ttl, &block) : block.call
   end

   def self.memcache
     @memcache ||= new_client
   end

   def self.new_client
     begin
       Dalli::Client.new(memcache_host)
     rescue StandardError
       false
     end
   end

   def self.memcache_host
     (production? || development?) ? 'cache.amazonaws.com:11211' : 'localhost:11211'    
   end

   def self.production?
     environment == :production
   end

   def self.development?
     environment == :development
   end

   def self.environment
     ENV['RACK_ENV']
   end
 end

Answer 3

I would do it like the following:

require 'dalli'
class Cache
    def self.fetch(key, ttl, &block)
        memcache and
            memcache.fetch(key, ttl, &block) or
            block.call
    end

    def self.host
        @host ||= (ENV['RACK_ENV'] == :production || ENV['RACK_ENV'] == :development) ?
            'cache.amazonaws.com:11211' :
            'localhost:11211'
    end

    def self.memcache   
        @memcache ||= Dalli::Client.new(host)
    rescue StandardError
        nil
    end
end

I prefer returning nil instead of false on methods that would otherwise return an object on success. If it returns false on failure, then it should return true on success, but that would then change the method's name to memcache.