How Not To Write An API

So a future version of Rails will get Pluggable Controller Caching. That's a good idea, and good on Rails for formalizing this extensibility. Better than monkey patching.

Looking at the implementation, though, leaves me cold. Let's look at a code snippet:


def read(name, options = nil)
super
@ether.get(name)
end


With some commentary:


Invoking super for each method call ensures that the proper messages get logged for each action


Wow, way to trust your users! </sarcasm> Requiring a call to super is very bad practice. Any time you are relying on a client of your class to call something, you're risking someone forgetting to make the call.

It's better to use the Strategy Pattern. Here, you finalize the API method (here it is read) and provide an abstract method or your subclasses. Marking the method as final (which I don't think you can do in Ruby, but I digress) ensures that a subclass can't override your functionality, and the abstract internal method ensures that your client doesn't have to call back to super.

Here's how I'd fix it:


def read(name, options = nil)
# some logging and error handling
read_internal(name, options)
end

protected

def read_internal(name, options = nil)
raise "Subclasses must implement this method"
end


Ta-da! No more forcing the user to call super, thus making your algorithm safe and well encapsulated.
2 comments

Popular posts from this blog