---
title: designing without ifs
teaser:
tags: web,good code
author: Jared Carroll
published_on: 2008-01-02
---

Alright here we go.

Say we got an app with groups, users and memberships.

Theres 2 different types of groups:

1. Public
1. Private

Now anyone can join a public group but in order to join a private group we have
to authenticate you against an external api.  For example, say there's a private
group called 'Yahoo' and in order to join it you have to have a Yahoo account
(yes this is nuts but hear me out).

I'm going to factor out the external <abbr title="Application Programming
Interface">API</abbr> code into a library and use ActiveRecord callbacks to
perform the validation.

Let's take a look at this.

```ruby
module PrivateGroupApi

  class Request

    attr_accessor :url

    def initialize(url)
      self.url = url
    end

    def authenticate(username, password)
      uri = URI.parse "#{url}?username=#{username}&password=#{password}"
      request = Net::HTTP::Get.new uri.path
      http = Net::HTTP.new uri.host, uri.port
      response = http.request request
      PrivateGroupApi::Response.new response.code
    end

  end

  class Response

    attr_accessor :code

    def initialize(code)
      self.code = code
    end

    def success?
      code == '200'
    end

    def failure?
      code == '404'
    end

    def error?
      code == '500'
    end

  end

end
```

Looks straightforward, a Request and a Response class.  The Request class takes
a url during construction and its #authenticate method takes a username and
password that will be looked up by the external api.  We'll make a requirement
saying a 200 is a valid account and a 404 is an invalid account.

Now for some usage.

```ruby
class Membership < ActiveRecord::Base

  belongs_to :group
  belongs_to :user

  validate_on_create :external_membership?

  attr_accessor :username,
    :password

  private

  def external_membership?
    if private?
      request = PrivateGroupApi::Request.new group.api
      response = request.authenticate username, password
      if response.failure?
        errors.add_to_base "You do not have an account with this group's web site"
      end
      if response.error?
        errors.add_to_base 'There was a problem when trying to
          authenticate your account'
      end
    end
  end

end
```

Ugh, 2 conditionals; testing for failure and testing for errors (network, <abbr
title="Application Programming Interface">API</abbr> errors, etc.).  Conditional
logic breeds bugs and always complicates things, I prefer for libraries and
frameworks to do the decision making for me.

Let's rewrite.

```ruby
module PrivateGroupApi

  class Request

    attr_accessor :url,
      :username,
      :password,
      :listeners

    def initialize(url)
      self.url = url
      self.listners = {
        '200' => lambda {},
        '404' => lambda {},
        '500' => lambda {}
      }
    end

    def authenticate
      uri = URI.parse "#{url}?username=#{username}&password=#{password}"
      request = Net::HTTP::Get.new uri.path
      http = Net::HTTP.new uri.host, uri.port
      response = http.request request
      listener = listeners[response.code]
      listener.call
    end

    def on_failure(&block)
      listners['404'] = block
    end

    def on_error(&block)
      listners['500'] = block
    end

  end

end
```

This redesign eliminated the need for a Response class.  Instead it uses an
event driven style with listeners that will be notified when a particular
response is given.

Now some usage again.

```ruby
class Membership < ActiveRecord::Base

  belongs_to :group
  belongs_to :user

  validate_on_create :external_membership?

  attr_accessor :username,
    :password

  private

  def external_membership?
    if private?
      request = PrivateGroupApi::Request.new group.api
      request.username = username
      request.password = password
      request.on_failure do
        errors.add_to_base "You do not have an account with
          this group's web site"
      end
      request.on_error do
        errors.add_to_base 'There was a problem when trying to authenticate
          your account'
      end
      request.authenticate
    end
  end

end
```

Thats much better.  We eliminated both conditionals by giving the library the
code to run when a certain response is given.  No more requiring users to make
decisions based on responses, resulting in much easier to read and less error
prone code.

That still leaves us with 1 conditional, which I don't care for, in the
`GroupMembership` model.  Look at the declaration of the `#external_membership?`
validation callback.

```ruby
validate_on_create :external_membership?
```

That is poor because to someone reading the code it seems like this validation
applies to all groups.  Only when you read the implementation of
`#external_membership?` do you see a conditional in there applying this
validation only to private group types.

Rails needs to give us the following:

```ruby
class Membership < ActiveRecord::Base

  belongs_to :group
  belongs_to :user

  validate_on_create :external_membership?,
    :if => lambda {|membership| membership.group.private?}

  attr_accessor :username,
    :password

  private

  def external_membership?
    request = PrivateGroupApi::Request.new group.api
    request.username = username
    request.password = password
    request.on_failure do
      errors.add_to_base "You do not have an account with this group's web site"
    end
    request.on_error do
      errors.add_to_base 'There was a problem when trying to authenticate
        your account'
    end
    request.authenticate
  end

end
```

And with that we eliminate the last conditional and the validation declaration
reads much more accurately if this is a membership for a private group, is its
username and password registered with its group's web site.  Rails has taken
care of the decision of whether to apply the validation or not.

I like this event driven style of design.  Web frameworks give us this when
handling requests e.g. in Rails your routes are mapped to controllers and
individual methods within a controller.  The framework is doing the decision
making for you when a particular request comes in.
