A Rant on the Misuse of Instance Variables

Earlier today at NYC Dev Shop we were looking at some code we are writing for interacting with Yodlee, a SOAP-based API that interacts with banks.

There are hundreds of API requests that can be made of the Yodlee API, and they all follow an identical pattern: You need to define four parameters (a service, an endpoint, an action, and the SOAP request body) and fire them at Yodlee.

So, being diligent Rubyists, we made a base class with subclasses to keep our code DRY.

module Yodlee
  class Base

    def fire options
      # This method takes the SOAP API request options, parses them, and sends
      # them to the Yodlee Server
      # Contents of this method are omitted, cause it's, ya know, irrelevant
    end

  end
end

module Yodlee
  class User < Base

    def register
      service   = "UserRegistrationService"
      namespace = "http://userregistration.usermanagement.core.soap.yodlee.com"
      action    = :register3
      body = {
        :key => 'etc etc',
        :another_key => 'bla bla'
      }

      response = fire(service: service, namespace: namespace, action: action, body: body)
    end

    def login
      service   = "LoginService"
      namespace = "http://login.login.core.soap.yodlee.com"
      action    = :login2
      body = {
        :key => 'etc etc',
        :another_key => 'bla bla'
      }

      response = fire(service: service, namespace: namespace, action: action, body: body)
    end

  end
end

So, coming back to the office discussion, we were looking for ways to make the code DRYer. We asked: Being that every single method is going to define the exact same four variables to send to the fire method, why don't we just assign them to instance variables? That way we don't have to send them as parameters. Like so:

# original implementation
def register
  service   = "UserRegistrationService"
  namespace = "http://userregistration.usermanagement.core.soap.yodlee.com"
  action    = :register3
  body = {
    :key => 'etc etc',
    :another_key => 'bla bla'
  }

  response = fire(service: service, namespace: namespace, action: action, body: body)
end

# with instance variables
def register
  @service   = "UserRegistrationService"
  @namespace = "http://userregistration.usermanagement.core.soap.yodlee.com"
  @action    = :register3
  @body = {
    :key => 'etc etc',
    :another_key => 'bla bla'
  }

  response = fire # look! no parameters 
end

The fire method could then access those four instance variables directly, as we are confident that every method that calls fire follows the same pattern and has defined those instance variables.

But there was something really bothering me about all this that I was having trouble articulating. Now that the words have found their way to my tongue, I figured I'd jot them down.

So What Are Those @ Thingies Anyway?

In my view, instance variables aren't just variables that happen to have object-wide scope. They are variables that exist to persist core elements of an object. They aren't intended for arbitrary data.

For example, if you had a user object, it would be more than kosher for it to have @first_name, @last_name, @company, and @errors variables. But a @thirty_ninth_last_thing_this_user_did instance variable? Not so much. The 39th last thing a user did is not a core part of a user object.

In our Yodlee code above, the four parameters in each method are exactly that - just four completely arbitrary parameters. In other words, it just so happens to be that if you want to register a user on Yodlee, you are looking for the :register3 action, but that particular action is not a core element of the Yodlee::User object.

Which is why assigning it to an instance variable just isn't right.

Diving In A Little

To take this a little deeper, the reason instance variables are not limited to local scope and are allowed to have object-wide scope is precisely because they contain core object data! But arbitrary parameters, on the other hand, should never have object-wide scope. They are not an integral part of the object.

If you do find yourself needing to bind arbitrary parameters to instance variables, take it as a sign that your object is modelled around the wrong thing.

To Wrap Up

So we reverted our code back to our original implementation. Although this involves passing in a hash of options to the fire method every time it is called, and the fire method in turn passes that options hash to yet more methods, it is nonetheless the right thing to do.