I worry about writing bad code and not knowing that it’s bad. So this week I took some time away from coding to read POODR and wrap my head around what “good code” even means.
There are many reasons to fall in love with the book, but I think one of the things Sandi Metz does best is show you what “bad code” looks like, and refactors it piece by piece, calling out different techniques she’s using along the way until she gets to her final solution.
The first time I read it, I was able to follow along with her analysis, but not really able to call out the techniques she talked about on my own. I understood but I didn’t retain.
So I read it again, taking the time to refactor her exercises before seeing her final solution, and then making my own “bad code” examples and seeing if I could talk through how and why it wasn’t written well, and how to improve it.
If you decide to do this yourself, I highly recommend reading and dissecting the book with someone else. I paired with the incredible Stephanie Oh, and I got a lot more out of it then I would have on my own.
After getting through half the book together, I was itching to start coding to see if I could apply what I’d just learned. My biggest goal wasn’t writing well designed code on the first go -- it was being able to recognize when the design of my code was headed in the wrong direction. I knew I could hack together a working app, but I wanted to be able to notice the red flags along the way that told me the design wasn’t well thought out -- when the relationships between the objects didn’t make sense, when I was writing way too much code for the task I was trying to accomplish, when changing my mind was painful. In the first app we built, these were all red flags I didn’t notice until the app was pretty much done -- I want to notice them sooner.
So I started working on a CMS for my personal blog. I knew exactly what it would need and what it would look like, so I started building.
The goal was to have a crazy simple interface where I could choose between 4 options: to look at my past posts, make a new blog post, make a new illustration, or edit my homepage info.
The feature I wanted to start on was creating a new blog post. So I spec-ed out that feature, jotting down the steps from the user’s perspective that I’d have to go through. The steps summed up as follows:
- Start at "/home"
- Click on “New Blog Post” link
- Fill out the form.
- Tadaaaa! You have a new blog post.
Simple enough. So I started building out that feature, letting the tests guide me. When I got to the routes, I had to pause: how do I account for the fact that I was dealing with three objects? I had blog posts and illustrations, which had their own attributes and would need different methods in the future. But they were both posts, and shared common attributes as well.
How would the database capture this relationship between the three? And how do I write controllers and actions that made the most sense? At what point did I tell my database that this wasn’t just a regular old post, but was actually a blog post?
I’d started out with just a single Post model, with a column name called post_type where I would specify whether it was a blog post or an illustration. I’d tried having a column name called “type” but Rails yelled at me, so I renamed it. I would soon find out why.
Following the tests, I found myself writing a route that made me feel uncomfortable: /posts/blogposts/new. Looking at that route, it seemed logical to create a corresponding BlogPostsController that handled all of the things I’d want to do with blog posts. And later, I would need to build out a corresponding IllustrationsController. This seemed like a way to separate the two in case I wanted to do very different things with them later. I continued building, realizing that although I had a BlogPostsController, I still had to tell my database what kind of post it was dealing with. I was sure this wasn't the right approach. I could see how messy this would get, but I wanted to play this out to clearly see and dissect the "bad code".
I built out the illustration feature in a similar fashion. And it worked, but it looked so wrong. I had three controllers and one model. Because the two objects had some matching attributes, their forms and all of their views were very similar and therefore very repetitive. And in my desire to keep these two things separate, you know, “just in case,” I found myself making unnecessary, premature decisions that weren’t helping the organization or maintainability of my code. And then the routes. There were so many routes. I really didn’t want to deal with all of those routes. The amount of code I was writing to do something pretty simple was quickly getting out of control.
So I paused and reexamined the overall architecture. The core problem was that I was trying to separate my blog posts from my illustrations. And instead of doing that in my models, where that distinction belonged, I was doing it through my controllers. I thought about how those three objects logically came together, you know, in real life, and it sounded a lot like single inheritance, a concept Sandi Metz talks about in POODR.
But how do you reflect single inheritance, where subclasses inherit from a single parent class, in a relational database? I started doing some research and came across Single Table Inheritance, a concept that Martin Fowler wrote about extensively in his book “Patterns of Enterprise Application Architecture.”
His solution is to create your parent class along with whatever subclasses you need:
post.rb class Post < ActiveRecord::Base end blog_post.rb class BlogPost < Post end illustration.rb class Illustration < Post end
You then create one table with column names representing attributes for both subclasses, and an additional column name called type. Hmm ... interesting. And that type is where the name of the subclass belongs. And that’s why Rails yelled at me -- because type is special. Type was how the table knew that those rows related to specific subclasses of a different name than the table name.
Now that I’d established these subclasses and gave the table a way to know what they were, I could get rid of my BlogPostsController and move any logic to my BlogPost model, where it belonged. I killed the routes, and rebuilt the feature with the same spec.
All went well until I was building out my form_for for a BlogPost object. In my PostsController, which was now my only controller, I made a new instance of the Post class.
post_controller.rb
class PostsController < ActionController
def new
@post = Post.new
@post.type = params[:post_type]
end
end
new.html.erb
form_for @post do | f |
end
Form_for looks for the class of the object to create the appropriate path. And although @post was an instance of the Post class, I’d assigned its type to “BlogPost” in the PostsController. So Rails, knowing that I meant for it to belong to the BlogPost subclass, was looking for a blog_post_path instead of a post_path. Aw crap.
How do I tell Rails to look for the parent class, Post, and build a post_path instead? After some googling, I found that building out the correct path relies on a method called model_name. So if I overwrote that method in the BlogPost model and redefined what the right class was, it would build the desired path.
So I wrote:
blog_post.rb
class BlogPost < Post
def self.model_name
Post.model_name
end
end
Using that, my form_for built the correct path and creating blog posts and illustrations was a success.
Looking at my code, I can still see places where refactoring are needed, and the design reworked a bit. There are a few if statements that are making me uncomfortable, but it’s feeling uncomfortable that I’m most excited about. It’s feeling like things have gone wrong while I’m building, rather than after the fact, that I’m excited of doing more of, even if I’m not always sure how to make it better.