More Tiny Classes

My last post, Keep ’Em Separated, made me realize I should start sharing more about what we are doing to make Gauges maintainable. This post is another in the same vein.

Gauges allows you to share a gauge with someone else by email. That email does not have to exist prior to your adding it, because nothing is more annoying that wanting to share something with a friend or co-worker, but first having to get them to sign up for the service.

If the email address is found, we add the user to the gauge and notify them that they have been added.

If the email address is not found, we create an invite and then send an email to notify them they should sign up, so they can see the data.

The Problem: McUggo Route

The aforementioned sharing logic isn’t difficult, but it was just enough that our share route was getting uggo. It started off looking something like this:

post('/gauges/:id/shares') do
  gauge = Gauge.get(params['id'])

  if user = User.first_by_email(params[:email])
    Stats.increment('shares.existing')
    gauge.add_user(user)
    ShareWithExistingUserMailer.new(gauge, user).deliver
    {:share => SharePresenter.new(gauge, user)}.to_json
  else
    invite = gauge.invite(params['email'])
    Stats.increment('shares.new')
    ShareWithNewUserMailer.new(gauge, invite).deliver
    {:share => SharePresenter.new(gauge, invite)}.to_json
  end
end

Let’s be honest. We’ve all seen Rails controller actions and Sinatra routes that are fantastically worse, but this was really burning my eyes, so I charged our programming butler to refactor it.

The Solution: Move Logic to Separate Class

We talked some ideas through, and once he had finished, the route looked more like this:

post('/gauges/:id/shares') do
  gauge    = Gauge.get(params['id'])
  sharer   = GaugeSharer.new(gauge, params['email'])
  receiver = sharer.perform
  {:share => SharePresenter.new(gauge, receiver)}.to_json
end

Perfect? Who cares. Waaaaaaaaay better? Yes. The concern of a user existing or not is moved away to a place where the route could care less.

Also, the bonus is that sharing a gauge can now be used without invoking a route.

So what does GaugeSharer look like?

class GaugeSharer
  def initialize(gauge, email)
    @gauge = gauge
    @email = email
  end

  def user
    @user ||= … # user from database
  end

  def existing?
    user.present?
  end

  def perform
    if existing?
      share_with_existing_user
    else
      share_with_invitee
    end
  end

  def share_with_existing_user
    # add user to gauge
    ShareWithExistingUserMailer.new(@gauge, user).deliver
    user
  end

  def share_with_invitee
    invite = ... # invite to db
    ShareWithNewUserMailer.new(@gauge, invite).deliver
    invite
  end
end

Now, instead of having several higher-level tests to check each piece of logic, we can just ensure that GaugeSharer is invoked correctly in the route test and then test the crap out of GaugeSharer with unit tests. We can also use GaugeSharer anywhere else in the application that we want to.

This isn’t a dramatic change in code, but it has a dramatic effect on the coder. Moving all these bits into separate classes and tiny methods improves ease of testing and, probably more importantly, ease of grokking for another developer, including yourself at a later point in time.

8 Comments

Nate Klaiber
Feb 06, 2012

Curious. You started with:

“because nothing is more annoying that wanting to share something with a friend or co-worker, but first having to get them to sign up for the service.”

And the end of that process being:

“If the email address is not found, we create an invite and then send an email to notify them they should sign up, so they can see the data.”

Aren’t you still forcing them to sign up for the service to see the data?

I agree with the other points. Over the past year people have been crying about “testing without Rails”. Their approach was to pretend Rails wasn’t there, even though they were actively using it. They just didn’t like how their tests were slow. The core of the problem is exactly what you describe here.

Take your application down to the smaller chunks, in some cases working only with primitives, and then use them as building blocks. Your code is easier to read and understand. Your code is easier to tests. In the case of Rails, you don’t always have to use or invoke the entire Rails stack, so your tests become naturally faster.

I call this good Ruby code :)
John Nunemaker
Feb 06, 2012

@Nate: The issue isn’t forcing them to sign up, but rather having to wait for them to sign up or coordinate it in some way. This keeps the process asynchronous. Your friend or co-worker can signup at their leisure.

The only thing I don’t like about this code is the name of the class is a verb, instead of a noun. I should probably think about it a bit more and change that.
Les Nightingill
Feb 06, 2012

just curious… where in the Rail directory structure do you like to stash little “helper” classes like this? /lib ? /app/domain_classes ? or do you put them alongside the ActiveRecord classes in /app/models ?
John Nunemaker
Feb 06, 2012

@Les: We are using sinatra. That said, anything directly related to this app I put in app/models. Anything that I write that could be or is used in other apps I put in lib. app/models is not just for database backed classes.
Brandon Hilkert
Feb 06, 2012

Really loved this post. Showed how simple some things can really be. The resulting code is really easy to follow.

Since you guys are using Sinatra for Gauges, I’d love to hear about your folder structure for a project and maybe a skeleton out there that you’d recommend. It obviously depends on the complexity, but I’m sure Gauges has a fair amount of code and am curious how you mange it in Sinatra.
John Nunemaker
Feb 07, 2012

@Brandon: Right now we use a typical folder structure for Rails. Most people would think it was a rails app at first class. We have app/models, app/presenters, app/views, and app/routes.
Brandon Hilkert
Feb 07, 2012

I assume you just have some iterator that loads all the contents of the app directory?

I think it would be an awesome post to talk about a formal structure because I’ve been finding a million different variants. I realize the point of Sinatra is to not necessarily have that, but given the speed of it for something simple like an API, I would be interested in seeing you load initialize the application to load all that content.
Daniel
Feb 20, 2012

Sorry to post off topic, but the comments are closed on the post explaining how to display git status in the bash prompt, and there is a typo that tripped me over.
Please fix it.
From git-symbolic-ref HEAD 2
to git symbolic-ref HEAD 2

Thanks.

Daniel