Thursday, December 27, 2012

Why I Don't Use ActiveSupport::Concern

or, why I actively replace ActiveSupport::Concern with Ruby in codebases I work on


tl;dr

I don't use ActiveSupport::Concern.
  • Unlike other Rails-based bastardization of concepts and names, ActiveSupport::Concern has very little to do with common connotations of "concern."
  • It tightly couples a useful design concept to a single, possibly-least-optimal implementation of the idea (mixins).
  • Its implementation is actual hiding a dependency management system. This hides design issues that are highlighted when you are more explicit.
However, if you want to use it, the world isn't going to end.

Background on why I noticed this

This summer, I spent some time at a local startup here in Chicago doing some work. I was brought in to build some features, look at their architecture, work on improving their test suite, etc. The system was young-ish (between 1- and 2-years old) and wasn't in bad shape. Features were still being added fairly readily (although starting to slow), and it wasn't TOO hard to find where things were. But the original developers had made some poor decisions, and you could see that if the same idioms were followed much more, the system would start to follow what is a common Rails progression, calcifying as heavy coupling starts to make it more and more difficult to make small changes without negatively influencing other parts.

Also, as their system grew, the lack of a good testing practice and structure slowed down their test time, both for the suite and (more importantly) the unit level. Heavy use of FactoryGirl-style integrated tests were making their test startup time less and less useful for the minute-to-minute development process. When running a single tests takes a long time, it is less likely that you'll run them frequently, so you lose one of the major benefits that a good test-driven process provides: design feedback as you flesh out your components, behaviors and interactions.

Again, their system wasn't in a horrible shape. I wasn't brought in to fix an already horrible situation. I was actually there to help keep them from going over the "super fast at first, then slows down tremendously" development cliff that haunts many of the Rails developers that I've talked to and spent time coding with.

They also had at least one missing domain concept in their system, as the original developers had waited a bit too long to focus on extracting the necessary design elements. This is a common problem I've seen: people (rightfully) wait to extract domain concepts until they present themselves, then miss the point in time when they can be easily extracted. So, it was a little bit past where reifying this fundamental concept was going to be a bit more work than they were comfortable taking. Since then, we've worked through a baby-step process to slowly bring the concept to life as a central figure in the design.

They were following a fairly idiomatic Rails design, focusing on putting business logic into their models. They tried to keep their controllers focused on the interaction between the user and the models. Of course, as many people who have worked with Rails know, this often leads to very large models. It also has a tendency to create complex interactions between your models. This results in large dependency graphs between objects, instead of a more streamlined tree-looking dependency diagram (also can be considered an acyclic graph). So, you have Model A depending on services from Model B, which depend on both Model A and Model C, which can lead to very complicated relationships and paths through your codebase.

The codebase had a combination of very large ActiveRecord model class files, as well as a hefty dose of extracted module code that was then added to the model via an include. The modules all used ActiveSupport::Concern to manage their inclusion, which is where this blog post comes from.

What is a concern?

One of the first steps I do when trying to pull apart large AR models is to group the methods into actual conceptual "concerns."  By moving these groups into modules and mixing them into the AR class, I can also get a good sense of the dependency graph between them. In general, I like these concerns to be orthogonal to each other. If I find that they are dependent on each other, it is usually an indication that there is a helper object of some sort waiting to be extracted.

Orthogonality of concerns


Although there are times when it can be the simplest, or at least an interim, solution, allowing a module to depend on another module should be treated as a design smell. This smell often flags a missing, but needed, service object. Unfortunately, idiomatic Rails design seems to shy away from more service-focused objects, relying on the "everything is in the AR model" design.

A good way to make sure that these concern modules are truly orthogonal is to mix them into your AR module without loading the other modules. This allows you to test just their code. If you find that you can't load them independently, then they have dependencies on each other and are perhaps hiding another concern. Loading modules in isolation is fairly easy to do, just create an empty version of the undesired modules before you require your AR module. Doing this allows you to run isolated tests of specific concern modules when doing development, but also allows you to have the same tests run in the context of loading all concern modules when running your whole suite. It works great, and I use this technique extensively.

NOTE: this requires an isolation-focus for testing your AR models. That is, you shouldn't need to load up your entire application just to test your AR models. You can see the way I do it here.

Isn't ActiveSupport::Concern just an alternate syntax for include/extend?

However, when I tried doing this on this specific codebase, the full suite wasn't working. The real versions of some of the modules weren't loading. Something was changing the way that Ruby's include was working. After some investigation, I narrowed it down to the modules that were relying on ActiveSupport::Concern, rather than just using Ruby's built-in include functionality. This shocked me, as I assumed that ActiveSupport::Concern was just an alternate syntax for Ruby's include/extend functionality. I was working with a pre-existing codebase, so I was just going by what I had read in blog posts, which emphasized the syntax. Now, personally, I dislike the syntax and find it to be one of the more unnecessary "syntactic sugar" pieces in an otherwise useful set of helpers provided with Rails.

Whoa! Dependency Management?

So, I went looking at the code for ActiveSupport::Concern. Go look at it, yourself, as well. ActiveSupport::Concern is focused on mucking around with the loading of modules in the case that you've allowed one module to depend on another. Having modules depending on other modules is a design smell, but, as with all design discussions, there are always some edge cases where this is needed and potentially the simplest solution. What caught me off guard, though, was that a component that was focused on managing a specific design was wrapped in a name that has nothing to do with that, namely "Concern."

Rails history of naming problems

Giles Bowkett has been writing some great stuff lately about Rails, OO and design. His book, Rails As She Is Spoke (yes, you should get it), has a good section on the problem with ActiveRecord as a name for the implementation in Rails. I won't go into too much detail, but it revolves around the misrepresentation of ActiveRecord, the ORM, and active record, the design pattern. Along with MVC, Controllers, Models, and pretty much the entirety of the "testing" names, Rails has had a history of bastardizing and confusing the meaning of already established terms.

It is common for Rails to be the first point of contact for developers with these terms. Many of the people who come to Rails either are beginners or come from programming cultures with less emphasis on software design and the history of where these terms come from. The result is that they now have a skewed view of these terms from the larger software industry as a whole. As an example, talk to a testing professional and use the terms that Rails uses for the different types of tests. You'll get some interesting looks.

Let's do the time-warp again

And now we have the term "concern" being bandied about. DHH recently wrote a reasonable blog post about separating out the different concerns of your AR models into modules. Unfortunately, he consistently confuses ActiveSupport::Concern (dependency management for module inclusion) with concern (orthogonal, possibly reusable modules containing conceptually related groups of functionality). By doing this, Rails once again is co-opting a term that is commonly used and has a well-established mechanism in Ruby (include/extend) and replacing it with a capitalized extra dependency. By presenting ActiveSupport::Concern as the same as "concern" the naming is again confused. And, unlike the other parts of Rails, this one has no added benefit related to even the most vague definition of concerns. In fact, it adds complexity to your system that wasn't there before when relying on include/extend.

It also tightly ties the idea of a "concern" to a concrete implementation in ActiveSupport. I use the idea of concerns a lot to guide me as I build the design for my systems. Sometimes the idea leads me to a module mixin, sometimes to a wrapper-type class, and sometimes to a helper class that I delegate requests to.

Do I need this dependency management functionality?

With the complexity (both inherent in the problem and caused by the choice of design) in Rails, I can see a need for a component that does what ActiveSupport::Concern does, managing module-to-module dependencies. But, why not have this as a separate, well-named component? On the surface, Rails purports to have a focus on usability, and it succeeds in a lot of areas, but historically it has messed up when it comes to names. While the other names at least have a passing similarity to their common connotation, though, ActiveSupport::Concern doesn't seem to have any actual link to anything but the vaguest of definitions of concern.

However, being explicit about this allows us to get necessary feedback from our design when we start creating strange dependency chains.

Adding complexity to Ruby's mixins


Because of the built-in dependency management overriding that this contains, you now have to jump through hoops to test ActiveSupport::Concern mixins in isolation of other mix-ins.

As an example, suppose you have the idea of authorization for certain behaviors. It could be argued (I've heard it, in fact) that authorization is a cross-cutting concern across models. So, why not implement this as a Concern? This is a reasonable idea, because you can then test that the authorization code is working, then declaratively lay it down on the rest of your system. However, this causes another small issue. In order to test the behavior of your component, you have to provide some form of setup around the authorization stuff. That adds unrelated complexity to your testing, masking the reason behind the test. Do this with enough "concerns" and you'll have a tremendous amount of cruft in your tests. Now, change one of them and all your tests break. Sound familiar? This is a common situation where the fragility of tests is a direct pointer to an implementation problem in your underlying design.

The current implementation of ActiveSupport::Concerns doesn't allow you to simply swap out the Authorization concern with a dummy implementation, due to its real nature as a dependency-management component.

And this is in place of Ruby's built-in mixin support with include/extend. Josh Cheek has a good blog post where he talks about include vs extend and some ways to use them.  Yes, you should go read it.

What it actually does

So, ActiveSupport::Concern appears to have 2 functions: an alternate syntax ("sugar" is a matter of opinion, I personally think it is less useful than Ruby's) for specifying how a module gets included into a class; and, a complex dependency management system (which has absolutely nothing to do with the concept of concerns).

But I like concerns

I love the idea of concerns. I use the concept all the time to guide my design decisions. In fact, I agree with DHH on a high level. If you replaced all references to ActiveSupport::Concern with just simple concern/include/extend in DHH's post about them, I think that his points are valid. In Rails, this is the place where you should be focused on separation of concerns. However, based on my experiences, mixins shouldn't be the primary mechanism for doing this in your system (I think wrappers or helpers are better way of eliminating duplication of knowledge/behavior), but it does fit with idiomatic "everything is in your model" Rails designs.

What should it be called?

ActiveSupport has a whole slew of useful additions to built in Ruby classes. Why not just have this be an augmentation of Module, focused on the dependency management?

Ultimate Question: should you use it?

I don't use them. I find that they add complexity to my system without providing any value. Adding dependencies and hiding complexity is a choice that you should make according to your desires and experience.

But, this blog post isn't about convincing you not to use them; it is about explaining the reasons why I don't use them. I prefer using Ruby's built-in, more flexible include/extend syntax. Your mileage may vary depending on your choice.

And, most importantly, the world won't end if you do. I would just ask yourself if it actually provides value. Rails has some awesome parts that make it very useful, but you should think before blindly using it for more than what it really is: a set of reusable helpers to route from a url to a method and a passably useful ORM. That part in-between those: that's your application, it is worth putting thought and care there.

Update: Other Options

People mentioned two alternatives to include/extend that don't have the naming- and dependency-management-craziness in them. Plus, the code is a bit more clear IMNSHO.
Shared by Ara Howard - The more complex of the two, supporting deferred execution of the code. Very interesting.
Augmentations by Henrik Nyh - Incredibly simple and focused.
Check them out, and especially check out the code! I'm not saying you should use them, but I definitely have a better feel about them than ActiveSupport::Concern.

And finally, here's a picture of Zak the Cat 
You just read a long blog post. You deserve to smile.




Thanks to Sarah Gray, Giles Bowkett and Thorsten Bรถttger for reviewing this.





Monday, December 17, 2012

Roman Numerals Kata with Commentary

I recently participated in an awesome Kata Battle at 8th Light against the worthy adversary, Craig Demyanovitch. The kata was the wonderful Roman Numerals Kata, converting arabic numbers to their roman numeral equivalent.

While practicing the kata, I noticed a lot of interesting things around the decisions I made in the solution. So, I thought I would record a katacast and lay over some voice commentary around it.

I'm putting two videos here, with and without commentary.

Here is the kata WITH commentary. It runs about 16 minutes and 40 seconds, so it isn't too bad to watch. I'd love to hear any constructive feedback in the comments. Version with no commentary is at the bottom of this post. I'd highly recommend watching it full-screen, so you can actually see it.



Resources:


Here it is with no commentary, if you just want to see me run through it at a reasonable speed.