A list of lessons learned from refactoring my open_auth2 library to make it a better OOP citizen.

open_auth2_refactor_lessons.md

Recently I've been learning a lot about OOP, especially how to structure code to make it easier to change in the future. The following are some of the ways I refactored my open_auth2 library.

Don't optimize prematurely

I did the mistake of assuming what sugars were required, only to realize I never used them. Source.

Inject dependencies

The Token object accesses Config object for information. Before it was hard coded into Token, so they're highly coupled, so now I pass it as an argument to #initialize. Better yet it defaults to OpenAuth2::Config.new so it's only optional. Source.

Trade offs between easeness and addtional dependencies

The gem had a simple plugin system where you can define providers that abstracted the idiosyncrancies of the various apis. The user can specify how an individual provider like say Facebook worked and then they'd tell the library to use the provider by specifying the name of it. Config would use active_support to find the class that corresponded to the name. active_support was not used in any other place

OpenAuth2::Config.new do |c|
  c.provider = :facebook
end

module OpenAuth2
  module Provider
    class Facebook
      # api specific config
    end
  end
end

I eventually decided to use full class names in Config itself. While a little verbose it cleaned up a lot of methods and more importantly removed active_support dependency.

OpenAuth2::Config.new do |c|
  c.provider = OpenAuth2::Provider::Facebook
end

module OpenAuth2
  module Provider
    class Facebook
      # api specific config
    end
  end
end

Simplify the public interface

client.get is a way to make GET request to an endpoint. Before it took a hash: client.get(:path => '/cocacola', :limit => 1) . Now client.get('/cocacola', :limit => 1) takes path as first argument and the rest of params as hash. It made sense for a client to take path as first argument and the rest as params. In hindsight I would add some kind of deprecation warning even though the gem is under 1.0, but the since the gem isn't very popular I didn't bother. Source.

Refactor names of methods, classes etc.

This is almost inevitable, as your knowledge and experience of the domain changes you'll get more ideas for better naming. I tried to clump them together to avoid messing my git commit history. Source.

Don't expose implementation

The gem uses Faraday to make the actual requests and the default providers hook into Faraday itself to change the params of the request. The reason this is very bad is now the user has to know about Faraday api to use this, ideally they should know about this library's public interface.

Take bite sized chunks

If your git status is like this while refactoring, it's probably time to stash it and start over.

Don't name your variables lowercases version of class names

hash is a terrible name for a variable, worse as an argument to a method. It tells nothing about what it is used for, only how it is implemented, possibly. A better name would be config or params.