# Using Dalli to connect to memcached

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

-

## 3 Answers

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

-
The begin and end keywords you get for free inside the block that is a method definition. –  vgoff Dec 24 '12 at 23:35
I'd change some indentation and blank lines, but basically this is it. One important thing though: dont' write rescue Exception, always rescue StandardError. –  tokland Dec 26 '12 at 19:56
Generally true, tokland, but Exception was what was given in the original question, so rather than assume they are rescuing the wrong thing, I stuck with it. It was not clear what they intended to rescue, though that is likely overreaching. I am sure it wasn't a comment to me, though, I am just saying. The indentation is only there to show a new user what the important part of the above line is, which is to indicate the two options below would be resulting in that parenthesis level. I tried to not affect behavior, only what they asked for. @gp, you want to answer about Exception choice? –  vgoff Dec 26 '12 at 22:04

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

-
I like it. Still the begin and end are implicit by virtue of being in a block, this block being the method definition. –  vgoff Dec 30 '12 at 8:26
I just don't like the un-indented rescue. Same as the indented or un-indented private statement, some like it one way, others like the other :) –  David Österreicher Dec 30 '12 at 13:26

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.

-
I've downvoted this answer. Cache::fetch and Cache::host are confusing. The the ternary disguised as and and/or is ambiguous ((a and b) or c or a and (b or c)). The ternary in Cache::host, like the one in Cache::fetch is so long, it's split it into multiple lines. They would both be clearer as regular if statements. Cache::memcache silently swallows every exception inheriting from StandardError, making debugging hard. Introducing Cache::host is a good decision, it makes Cache::memcache clear and concise. –  britishtea Jan 22 at 23:24
If you know Ruby, you should know that the and and or operators behave similarly to bash's && and ||. So order of operation should be a non-issue. It's like saying (5 * 3) + 2. Redundant. Long ternary operations are non-issue. If we look at David's answer, where he split the env check into their own functions, it can be a good idea if the rest of the system also could take advantage of the env check. But from the question alone, we don't know if that's even needed. –  DumpHole Jan 23 at 3:03
The precedence rules for and/or are unintuitive (and, more importantly, different from the more popular &&/||!). I have to look them up all the time. I had to look quite a bit longer at the and/or construction in Cache::fetch to recognize it as a simple if statement. An if statement would have made it crystal clear that it is an if statement. Fun example: a = nil or "foo" (a is now nil, but the return value is "foo"). –  britishtea Jan 23 at 9:06