Is it cute or ugly?

Yesterday day I presented a small code snippet to my dear coworkers (who I of course always respect) and (as usually) they were not impressed, even though I liked my solution. So I wanted to know your opinion about this little piece of code. Let’s start with the beginning.

Obvious way

The story is simple. You are writing code to communicate with external api. You are thoughtful programmer so you don’t store credentials in the code. You deploy to Heroku so obviously you keep things in ENV .

First thought:

class ApiProvider def initialize ( login = nil , password = nil ) login ||= ENV [ 'APIPROVIDER_LOGIN' ] password ||= ENV [ 'APIPROVIDER_PASSWORD' ] @uri = Addressable :: URI . parse ( "http://api.example.org/query" ) @uri . query_values = { usr: login , pwd: password } end end

My immediate concern: Why should the instance of my class know in its constructor about the fact that we use ENV to store default login and password values. So perhaps we should use some kind of factory that would create the object with provided values or the defaults ?

But in Ruby every class is a factory, so why not use is to our advantage…

Introduce Factory

class ApiProvider def self . new ( login = nil , password = nil ) login ||= ENV [ 'APIPROVIDER_LOGIN' ] password ||= ENV [ 'APIPROVIDER_PASSWORD' ] super ( login , password ) end def initialize ( login , password ) @uri = Addressable :: URI . parse ( "http://api.example.org/query" ) @uri . query_values = { usr: login , pwd: password } end end

There is still something wrong here, I think. Why would anyone want to provide login, but not password ? Or password without login ? Doesn’t make much sense to me. So I decided to extract a new, little class.

Extract class

class ApiProvider class Credentials < Struct . new ( :login , :password ) end def self . new ( credentials = nil ) credentials ||= Credentials . new ( ENV [ 'APIPROVIDER_LOGIN' ], ENV [ 'APIPROVIDER_PASSWORD' ] ) super ( credentials ) end def initialize ( credentials ) @uri = Addressable :: URI . parse ( "http://api.example.org/query" ) @uri . query_values = { usr: credentials . login , pwd: credentials . password } end end

Does it make sense here to use Struct ? I think so, because Credentials.new('l', 'p').should == Credentials.new('l', 'p') . But there are coworkers who disagree with me and I wonder what you think.

Alternatives

Default in method definition

class ApiProvider def self . new ( credentials = Credentials . new ( ENV [ 'APIPROVIDER_LOGIN' ], ENV [ 'APIPROVIDER_PASSWORD' ] )) super end end # or class ApiProvider def initialize ( credentials = Credentials . new ( ENV [ 'APIPROVIDER_LOGIN' ], ENV [ 'APIPROVIDER_PASSWORD' ] )) @uri = Addressable :: URI . parse ( "http://api.example.org/query" ) @uri . query_values = { usr: credentials . login , pwd: credentials . password } end end

Somehow this seems to be less readable to me

Moving the defaults to Credentials

class ApiProvider class Credentials < Struct . new ( :login , :password ) def self . default new ( ENV [ 'APIPROVIDER_LOGIN' ], ENV [ 'APIPROVIDER_PASSWORD' ] ) end end def initialize ( credentials = Credentials . default ) @uri = Addressable :: URI . parse ( "http://api.example.org/query" ) @uri . query_values = { usr: credentials . login , pwd: credentials . password } end end

Nice, but the knowledge about defaults was transffered from ApiProvider.new factory method to Credentials and I believe that Credentials should but just a dumb class responsible only for keeping login and password always together. Because in terms of this api it never makes sense to operate separately on them.

External context is always responsible for providing the configuration

api = ApiProvider . new ( ENV [ 'APIPROVIDER_LOGIN' ], ENV [ 'APIPROVIDER_PASSWORD' ] ) api . do_something

This leads to repeated code if there are multiple places that need to instantiate ApiProvider .

TLDR

The constructor states that ApiProvider always requires credentials as dependency for proper working

class ApiProvider def initialize ( credentials ) @uri = Addressable :: URI . parse ( "http://api.example.org/query" ) @uri . query_values = { usr: credentials . login , pwd: credentials . password } end end

ApiProvider.new factory method is responsible for creating ApiProvider instance even without explicit credentials because defaults can be used.

class ApiProvider def self . new ( credentials = nil ) credentials ||= Credentials . new ( ENV [ 'APIPROVIDER_LOGIN' ], ENV [ 'APIPROVIDER_PASSWORD' ] ) super ( credentials ) end end

Credentials is just a dumb struct for passing login and password together around the system

class Credentials < Struct . new ( :login , :password ) end