Extracting the InviteRedeemer class from the Invite model in Discourse – Part 1

After poking around the Discourse project on Code Climate, I found that the Invite#redeem method was given a high complexity score.

Starting Grade

After studying the redeem method for a bit, I decided it would be a good subject for the next refactoring. The method that we started with is shown below.

While looking at the original method, there are a few things I saw that guided this refactoring.  The first is that the class is named Invite but the redeem method performs a lot of actions that go above and beyond what would be considered a single responsibility.

Invite shouldn’t care about how it is redeemed, it just needs to know if it has been redeemed or not.  With this as the starting point, I created a new InviteRedeemer struct that is called from within the Invite#redeem method.

The Invite#redeem method becomes the following:

To get the functionality moved, copy the contents of the redeem method from Invite to InviteRedeemer.  Running the tests will guide you to the next few steps which will be to preface any method calls that were previously called on the invite object itself with the invite attribute that is passed into InviteRedeemer. Repeat this until all of the tests are green again.

Now that the functionality is moved and all the tests are green, the process of refactoring the internals of the InviteRedeemer#redeem method can begin. This is where I’ll pick up in the next post.

But What About Testing?

In the last post, I didn’t mention testing but please don’t take that to mean that no testing was done.  I ran tests as soon as I forked the repository and after each small change.  Like I heard at the recent Ancient City Ruby conference, you don’t want to stay in the red too long.

While I was living in the .Net world, testing was always something that I wanted to do but it just isn’t as easy as it is in RoR.  My testing in .Net never seemed to gain traction.  This is another reason why my shift to Ruby makes me feel like I’ve landed in the right spot.  It’s just too easy not to do.

I really don’t like getting into the tool/framework debate, so my take is to pick a testing framework that works for you and get to work.  If you can grasp it quickly and it just makes sense, then go for it and stick with it.

Removing Duplication From the Post Model in Discourse

Discourse made a big splash a few months back when the project was first revealed.  Being that it’s such a well known codebase in the Ruby community I decided to spend some time doing some minor refactoring to get my feet wet.

After looking at the scores on Code Climate, the Post model seemed like a good place to start.  The code analysis showed that a few methods were increasing the duplication score.

Post Duplication

Post Duplication

The code in question are the three methods below:

At first glance, the repetitive check on the user jumped out at me.  This is easily solved by introducing a private method that does the check and has a name that clearly conveys the intention.

  def acting_user_is_trusted?
    acting_user.present? && acting_user.has_trust_level?(:basic)
  end

After the extraction, the checks in the methods were changed to call the private method.

With the user check taken care of, the next step is to replace the calls to errors.add with a method that simplifies the addition of the errors.  To do this, I added another private method.

With the new private method to add errors in place, we now have everything we need to finalize our refactoring.  Many of the methods that were flagged in the duplication check are now one line and have calls to method names that make it easier to grasp what is being accomplished.

After this refactoring, another check was run through Code Climate and the result is that the duplication is now removed.

after-refactor

This change was pushed to Discourse and accepted in commit a866463.

The Shift

In November 2011, I took a Ruby on Rails Pragmatic Studio class taught by Jim Weirich and Dave Thomas where I learned just how different things could be when you’ve been a Microsoft guy all of your career. While I will continue to use .Net and other Microsoft products on a regular basis, I feel that Ruby and Rails seem to be a near perfect fit for the projects I work on everyday.

The community around Ruby is phenomenal and this will be my little piece to add. This site will be a collection of Ruby, Rails and refactorings with a focus on refactoring code that could be improved or changed based on the Code Climate score. I’m a firm believer in that you have to track something to make it better and I feel this is the most open way to do just that while improving my own skills and hopefully helping others do the same.