Isnor Creative
Isnor Creative Blog
Ruby, Ruby on Rails, Ember, Elm, Phoenix, Elixir, React, Vue

Mar 23, 2015

Code smell – Double dipping Ruby on Rails controller actions

Something that I’ve encountered on some Rails projects that I have inherited, or done work on is single controller actions that serve multiple request types – for example, a /contact route that handles both initially displaying a contact form, via GET request and then a subsequent form post, via POST request.

I feel this is code smell. Intuitively, it feels like a something brought over from PHP or perhaps other pre-Rails web development appraoches; I seem to remember, that when I was learning PHP back in the day, I may learned that approach myself from PHP books. There are many potential problems with this approach, though, in my estimation.

Fat Model, Skinny Controller

Fat Model, Skinny Controller – this concept and the logic that follows from it, encourages organized code, by refactoring bloated controller actions into models, services, concerns, etc. When you have a single action handling your GET and POST, you’ve already probably doubled the size of it. And for what benefit?

It’s not RESTful

It’s not RESTful – if you’ve got these double-dipping actions in place, you may be missing an oportunity to work within the Rails way, and the RESTful approach. It may be a good idea to look at the action and the resource that it represents, and find out why you aren’t handling this with a resources :foo. If you have this sort of action in place because you’re working with a non-Active Record situation, take a look at moving to ActiveModel, which can function nicely as a resource.

Subject to change with updates

If you don’t have adequate test coverage this will bite you in the ass, if you do, you’ll still need to update your code and tests when updating Rails. Rails updates have meant that you can do longer simply say match ‘/contact’ => ‘contact#index’ and then have that route accept GET and POST requests. You’re going to have to update every route that uses this approach to something like match ‘/contact’ => ‘contact#index’, via: [:get, :post], Rails 4.0 also made changes such that PUT requests became PATCH requests. If your code is checking request.put? you will need to update your tests and controller logic to account for it. And god help you if you don’t have adequate test coverage in place.

I’m sure there are other problems with this approach that I can’t think of off the top of my head, or others still that I’m not even aware of. But one thing I know for sure, this has a distinct aroma of code smell in my opinion, and I encourage you to either split these double-dippers into discrete actions, or to rethink what you are representing and whether it can become a resouce.

I am available for Ruby on Rails consulting work – get in touch to learn more.

Gordon B. Isnor

Gordon B. Isnor writes about Ruby on Rails, Ember.js, Elm, Elixir, Phoenix, React, Vue and the web.
If you enjoyed this article, you may be interested in the occasional newsletter.

I am now available for project work. I have availability to build greenfield sites and applications, to maintain and update/upgrade existing applications, team augmentation. I offer website/web application assessment packages that can help with SEO/security/performance/accessibility and best practices. Let’s talk

comments powered by Disqus