Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RailsConf: Concerns #9

Open
wants to merge 2 commits into
base: railsconf-start
Choose a base branch
from
Open

RailsConf: Concerns #9

wants to merge 2 commits into from

Conversation

justin808
Copy link
Member

RailsConf 2014 Talk, Tuesday, 2:30, Ballroom 4, Concerns, Decorators, Presenters, Service Objects, Helpers, Help Me Decide!

Rails Concerns

Code: Github Fat Code Refactoring Techniques Pull Request Concerns
Example: Breakout Concern Emailable from User model.

Why?

  1. Your model file is huge, and your matching spec is even larger.
  2. You've got duplicated code in several models
  3. You want a super easy and clean way to put this code into a single module containing:
  4. Instance methods
  5. Class (static) methods
  6. Class macros (has_many, belongs_to, validates, scope, etc.)

What?

  • Simple, safe, easy refactoring.
  • Default part of Rails, not an extra gem.
  • Works not just for models, but also controllers.
  • Not just for code shared across models, but super to simply divide
    a large model file into logical chunks with associated tests.
  • This is best shown with this simple example based @dhh's article Put chubby models on a diet with concerns:
    module Taggable
      extend ActiveSupport::Concern

      included do
        has_many :taggings, as: :taggable, dependent: :destroy
        has_many :tags, through: :taggings 
      end

      def tag_names
        tags.map(&:name)
      end

      module ClassMethods
        def tagged_with(tag)
          # implementation for creating relation
        end
      end

      private
        def some_private_method
          # code
        end
    end

How?

  • Discover a set of related behavior in a model. Possibly, this behavior is in
    multiple models. Prefer to discover a "domain" rather than some "technical"
    aspect. A domain grouping is like Taggable. A technical grouping would be
    like FinderMethods or "ValidationMethods"
  • Create a file to hold the concern module:
  • If just one concern for model, or if shared among models, then in
    app/models/concerns.
  • If multiple concerns for single model, group in subdirectory under
    models, such as app/models/user/.
  • In the new module, underneath the module declaration, place extends ActiveSupport::Concern. RubyMine has a nice default file template.
  • Cut and paste relevant code from model into your concern.
  • Just create the concern file, cut and paste the code into the right spots,
    and include the concern file in the model.
  • Reference: Concerns API

Steps

  • Create file for Concern with skeleton structure in
    /apps/models/concerns/model_name. Omit sub-directory and ModelName:: if
    only one concern for given model or a shared concern.
        module ModelName::ConcernName
          extend ActiveSupport::Concern

          included do
            # your class macros
          end

          # place your instance methods here

          module ClassMethods
            # place class methods here, removing self.
          end
        end
  • Move instance methods to module.
  • Move class macro code to included block.
  • Move class methods to inside of module ClassMethods.
  • Place include statement in original model class:
        include ModelName::ConcernName
  • Move spec code out of /spec/models/model_name_spec.rb and into
    /spec/models/concerns/model_name/model_name_spec.rb or
    /spec/models/concerns/concern_name_spec.rb depending on how you created your
    concern.

Advantages

  1. Ease and safety of refactoring. Concerns are a great first refactoring step
    because using concerns involves simply moving the methods and tests into
    separate files. And code accessing those methods does not need to change.
  2. A core part of Rails 4, so one can expect familiarity among Rails
    developers.
  3. Simplicity. It's just a code organization that makes it easier to navigate to
    the right source and test file. Simpler than plain Ruby methods of include
    and extend.
  4. Can DRY up code when a concern is shared among multiple models or controllers.

Disadvantages

  1. The model object does not become more cohesive. The code is just better
    organized. In other words, there's no real changes to the total public API
    surface area of the model.

Alternatives

  1. Use plain Ruby and simply include or extend another Module.
  2. For a complex operation involving many different methods, and possibly
    multiple models, you may create non-ActiveRecord models, especially in cases
    where the functionality crosses multiple models.
  3. If the code is presentation-centric, and/or relies on helpers, then a
    Decorator is a better choice. See Draper Decorators.

Notes

  • Nesting concerns (concerns loading concerns) works, so you can break up a
    concern into smaller files by having a parent concern include several child
    concerns. See the Concerns API for an example.
  • Applies to controllers as well as models, although with controllers, on can
    break up a controller into several controllers that are referenced in the
    routes.rb file, so you may not need to use a Concern for simply reducing
    the size of a controller. In the controller case, concerns are useful for
    sharing code without using inheritance.

Concerns References

Review on Reviewable

@justin808 justin808 changed the title RailsConf Concerns RailsConf: Concerns Apr 18, 2014
@panozzaj
Copy link

Got here from your blog post about this presentation. I really like this style of showing diffs for each type of refactoring. Curious: what are your thoughts on changing the tests to 1) test the Emailable concern with a generic object as opposed to User and 2) asserting that the User class uses the Emailable concern? I have used this pattern in the past and it seems similar, but considering also that your approach has as much value and is a little simpler to reason about. Thanks!

@justin808
Copy link
Member Author

@panozzaj I think your approach might be good once the concern is used among multiple models/controllers. There's a couple of ways to do "shared" tests in rspec. I've tended to test both objects directly. I prefer simpler testing, even if a little slower.

@panozzaj
Copy link

Cool, makes sense. Kind of YAGN extraction until you get two models. Thanks
for the quick reply!

Sent from my phone
On Apr 29, 2014 12:40 PM, "Justin Gordon" notifications@github.com wrote:

@panozzaj https://github.com/panozzaj I think your approach might be
good once the concern is used among multiple models/controllers. There's a
couple of ways to do "shared" tests in rspec. I've tended to test both
objects directly. I prefer simpler testing, even if a little slower.


Reply to this email directly or view it on GitHubhttps://github.com//pull/9#issuecomment-41722339
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants