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.

* Interested in seeing an application being built with discussions at this level? Check out my video series: "Build an App with Corey Haines" on CleanCoders.com *

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.

* Interested in seeing an application being built with discussions at this level? Check out my video series: "Build an App with Corey Haines" on CleanCoders.com *

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.





17 comments:

  1. I totally agree with you that naming in various places of Rails is problematic and this one is a doozy. I do use it, however, and in my mind's eye I just see include/extend (dare I say multiple inheritance?) wherever I see ActiveSupport::Concern. I know it's got nothing to do with concerns so it doesn't cause me friction. But, I do take your point that overloaded and mangled terminology is not helping newcomers to Rails.

    I don't have the same attitude to module dependencies that you do, and perhaps that's why I'm more relaxed about this issue. I'm concerned that you label module dependency a "design smell". I'm quite concerned about it. While a design smell is potentially a surface indication of a deeper design problem, it seems to me that common use connotes that "you should never do that".

    Let's compare module dependency to, say, a long method. Long methods almost always indicate a design problem. I don't think Module dependency is on the same scale. I concede that it could be a bad design in a given context, and it's often used badly. But if you are concerned about misleading new Rails developers with confusing terminology, you should also be concerned with their perception when you say something is a design smell.

    Perhaps we need another term.

    ReplyDelete
    Replies
    1. I've faced a lot of gnarly long methods, and module dependencies scare me more than they do. As desigb problems go, long methods are pretty obvious. Module dependencies are more insidious, and their long-term effect on a codebase is further-reaching.

      Delete
  2. Hi, Mark, thanks for the comment. I can see what you mean.

    I like the term smell, because it is an indication that something *might* be (and often is) a problem. There are cases where module-to-module dependency is a valid solution, but, just like with other smells (circular dependencies, for example), it should be a flag that directs your attention to it.

    In general, I consider smells to be indicative of decisions that *should* be justified, rather than taken as an unspoken "this is okay." I think this coincides with the idea that it is "always bad, so I should justify if I do it." As with every "rule," though, there are exceptions. You just need to think and justify them. So, in the case of module-to-module dependency, I think you definitely should spend some time thinking about whether it really is a better solution than a separate helper object.

    ReplyDelete
    Replies
    1. Yes, that expresses it nicely ... and yes, I do spend time thinking.

      Delete
    2. :) Whoa, I just re-read that and the "you" might have implied you as in Mark. :) Sorry about that. Meant more the "you" as a general pronoun.

      Delete
    3. It's OK ... I did take the right way, but since it was ambiguous, just pushed back a little :-). I think these are important things to talk about and I realie I've been lurking around the Rails community for too long!

      Delete
    4. Whew! I'm glad.

      Given that tone is so important and easily misunderstood online, giving a little push here and there is a great thing.

      Delete
  3. I am starting to build a Rails app and have been reading a lot these last couple of days about concerns, refactoring, service objects, avoid putting all the business logic on the model, etc.

    You also mention that the "common Rails progression, calcifying as heavy coupling starts to make it more and more difficult to make small changes without negatively influencing other parts."

    I am not sure how I could break up my models into service objects, maybe it is because it's simply not at that level of complexity yet?

    However, I would like to know if there are any resources that I could look at to see examples of refactoring, service objects and other strategies to avoid the heavy coupling you mention.

    Thanks

    ReplyDelete
    Replies
    1. Hi, Alex,

      I'd check out books like
      Objects on Rails http://objectsonrails.com/
      Practical Object-Oriented Design in Ruby http://www.amazon.com/Practical-Object-Oriented-Design-Ruby-Addison-Wesley/dp/0321721330/ref=sr_1_1?ie=UTF8&qid=1356896969&sr=8-1

      Also, Gary Bernhardt's Destroy All Software screencasts are great. https://www.destroyallsoftware.com/screencasts

      Delete
  4. I'd like to see you lead an open space at CodeMash about how to better structure your AR models (e.g. yeah, we know we don't like big AR models, but what are our other options?). Seems like there could be some good discussion about this.

    ReplyDelete
  5. That would be fun. I actually have 3 primary ways that I break up my AR models (including mixins), whether they fall under the idea of "concerns" or just collaborators.

    ReplyDelete
  6. Corey - Great post. At my work we have a codebase that sounds very similar to what you described above. I'm particularly interested in your initial statements: `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.`.

    We have a number of models that might benefit from your approach. Do you have any examples that show how you did this?

    Thanks!
    Chip

    ReplyDelete
    Replies
    1. Hi,

      I don't, as the codebases that we've done this on tend to be proprietary. It is an interim step usually to pulling out objects that represent services.

      On thing I do is physically group methods in my models by the data they deal with. This can a reasonable first-order approximation for your concerns. If you notice a big clumping of methods around a particular piece of data, then you can sometimes see relations between them.

      In general, when you have these big objects, you can find clumps of behavior that really want to be extracted. Extracting to a module is a fairly safe interim step to moving it into a service object.

      I plan to blog more about some of these ideas. I'm releasing the first episode of a video series next week "build an app with corey haines," and it will cover some ideas like this. It builds an application from the ground-up, though, so won't necessarily be doing too much of the group, extract to module, then extract to class refactoring that is common on an existing, larger codebase.

      -Corey

      Delete
  7. Really good post, and I've applied a lot of principles of proper dependency injection in PHP Laravel projects. I've done some blog posts on not putting what I refer to as "global" functionality in your models the way Rails seems to encourage.

    I'll be looking into my options for dependency injection and how to write services in Rails over the next bit.

    ReplyDelete

Please don't post anonymously; I won't use your name/url for anything insidious.
Comments by Anonymous are not guaranteed to make it through moderation.
Constructive comments are always appreciated.