For the first time in quite a while, I’ve been able to spend time working on a brand-new Rails application that’s actually a business thing and not a side project. It’s small. Okay, it’s really small. But at least for the moment it’s mine, mine, mine. (What was that about collective code ownership? I can’t hear you…)
This seemed like a good time to reflect on the various Object-Oriented Rails discussions, including Avdi’s book, DCI in Rails, fast Rails tests, Objectify, DHH’s skepticism about the whole enterprise, and even my little contribution to the debate. And while we’re at it, we can throw in things like Steve Klabnik’s post on test design and Factory Girl
I’m not sure I have any wildly bold conclusion to make here, but a few things struck me as I went through my newest coding enterprise with all this stuff rattling around in my head.
A little background — I’ve actually done quite a bit of more formal Object-Oriented stuff, though it’s more academic than corporate enterprise. My grad research involved teaching object-oriented design, so I was pretty heavily immersed in the OO documents circa the mid-to-late 90s. So, it’s not like I woke up last May and suddenly realized that objects existed. That said, I’m as guilty as any Rails programmer at taking advantage of the framework’s ability to write big balls of mud.
Much of this discussion is effectively about how to manage complexity in an application. The thing about complexity, while you can always create complexity in your system, you can’t always remove it. At some point, your code has to do what it has to do, and that’s a minimum on how complex your system is. You can move the complexity around, and you can arguably make it easier to deal with. But… to some extent “easier to deal with” is subjective, and all these techniques have trade-offs. Smaller classes means more classes, adding structure to make dependencies flexible often increases immediate cost. Adding abstraction simplifies individual parts of the system at the cost of making it harder to reason about the system as a whole. There are some sweet spots, I think, but a lot of this is a question of picking the Kool-Aid flavor you like best.
Personally, I like to start with simple and evolve to complex. That means small methods, small classes, and limited interaction between classes. In other words, I’m willing to accept a little bit of structural overhead in order to keep each individual piece of the code simple. Then the idea is to refactor aggressively, making techniques like DCI more something I use as a tool when I see complexity then a place I start from. Premature abstraction is in the same realm as premature optimization. (In particular, I find a lot of forms of Dependency Injection really don’t fit in my head, it takes a lot for me to feel like that flavor of flexible dependencies are the solution to my problem.)
I can never remember where I saw this, but it was an early XP maxim that you should try to keep the simplicity 90% of your system that was simple so that you had the maximum resources to bear on the 10% that is really hard.
To make this style work, you need good tests and you need fast tests — TDD is a critical part of building code this way. You need to be confident that you can refactor, and you need to be able to refactor in small steps and rerun tests. That’s why, while I think I get what Gregory Moeck is saying here, I can’t agree with his conclusion. I think “more testable” is just as valid an engineering goal as “fast” or “uses minimal memory”. I think if your abstraction doesn’t allow you to test, then you have the wrong abstraction. (Though I still think the example he uses is over built…).
Fast tests are most valuable as a means to an end, with the end being understandable and easily changeable code. Fast tests help you get to that end because you can run them more often, ideally you can run them fast enough so that you don’t break focus going back and forth between tests and code, the transition is supposed to be seamless. Also, an inability to write fast tests easily often means that there’s a flaw in your design. Specifically, it means that there’s too much interaction between multiple parts of your program, such that it’s impossible to test a single part in isolation.
One of the reasons that TDD works is that the tests become kind of a universal client of your code, forcing your code to have a lot of surface area, so to speak, and not a lot of hidden depth or interactions. Again, this is valuable because code without hidden depth is easier to understand and easier to change. If writing tests becomes hard or slow, the tests are trying to tell you that your code is building up interior space where logic is hiding — you need to break the code apart to expose the logic to a unit test.
The metric that matters here is how easily you can change you code. A quick guide to this is what kinds of bugs you get. A well-built system won’t necessarily have fewer bugs, but will have shallower bugs that take less time to fix.
Isolation helps, the Single Responsibility Principle helps. Both of these are good rules of thumb in keeping the simple parts of your code simple. But it also helps to understand that “single responsibility” is also a matter of perspective. (I like the guideline in GOOS that you should be able to describe what a class does without using “and” or “or”.
Another good rule of thumb is that objects that are always used together should be split out into their own abstraction. Or, from the other direction, data that changes on different time scales should be in different abstractions.
In Rails, remember that “models” is not the same as “ActiveRecord models”. Business logic that does not depend on persistence is best kept in classes that aren’t also managing persistence. Fast tests are one side effect here, but keeping classes focused has other benefits in terms of making the code easier to understand and easier to change.
Actual minor Rails example — pulling logic related to start and end dates into a
DateRange class. (Actually, in building this, I started with the code in the actual model, then refactored to a
HasDateRange service module that was mixed in to the ActiveRecord model, then refactored to a
DateRange class when it became clear that a single model might need multiple date ranges. The
DateRange class can be reused, and that’s great, but the reuse is a side-effect of the isolation. The main effect is that it’s easier to understand where the date range logic is.
I’ve been finding myself doing similar things with Rails associations, pulling methods related to the list of associated objects into a
HasThings style module, then refactoring to a
You need to be vigilant to abstractions showing up in your code. Passing arguments, especially if you are passing the same argument sets to multiple methods, often means there’s a class waiting to be born. Using a lot of
If logic or
case logic often means there’s a set of objects that have polymorphic behavior — especially if you are using the same logical test multiple times. Passing around
nil often means you are doing something sub-optimally.
Another semi-practical Rails example: I have no problem with an ActiveRecord model having class methods that create new objects of that model as long as the methods are simple. As soon as the methods get complex, I’ve been pulling them into a factory class, where they become instance methods. (I always have the factory be a class that is instantiated rather than having it be a set of class methods or a singleton — I find the code breaks much more cleanly as regular instance methods.) At that point, you can usually break the complicated factory method into a bunch of smaller methods with semantically meaningful names. These classes wind up being very similar to a DCI context class.
Which reminds me — if you are wondering whether the Extract Method refactoring is needed in a particular case, the answer is yes. Move the code to a method with a semantically meaningful name. Somebody will be thankful for it, probably you in a month.
Some of this is genuinely subjective — I never in a million years would have generated this solution — I’d be more likely to have a Null Object for Post if this started to bother me, because event systems don’t seem like simplifications to me.
I do worry how this kind of aggressive refactoring style, or any kind of structured style, plays out in a large team or even just a team with varying degrees of skill, or even just a team where people have different styles. It’s hard to aggressively refactor when three-dozen coders are dependent on something (though, granted, if you’ve isolated well you have a better shot). And it’s hard to overstate the damage that one team member who isn’t down with the program can do to your exquisite object model. I don’t have an answer to this, and I think it’s a really complicated problem.
You don’t know the future. Speculation about reuse gains and maintenance costs are just speculation. Reuse and maintenance are the side effect of good coding practices, but trying to build them in explicitly by starting with complexity is has the same problems as any up-front design, namely that you are making the most important decisions about your system at the point when you know the least about the system. The TDD process can help you here.