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

Earlier Draft: Concerns #3

Open
wants to merge 3 commits into
base: refactoring
Choose a base branch
from
Open

Earlier Draft: Concerns #3

wants to merge 3 commits into from

Conversation

justin808
Copy link
Member

Rails Concerns

Why Concerns and Not Plain Ruby

  • Simple to consistently follow the pattern
  • Concerns can include other concerns
  • Single concern includes all of:
    • instance methods
    • class methods
    • code to run on model including the concern, such as =has_many=, =scope=,
      etc.
  • Yes, you can do the same with include, extend, etc., but why risk the
    pitfalls and why not the consistency of Concerns?

Where to put concerns

  • 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/.

Steps

  • Create file for Concern with skeleton structure:
  module ModelName::ConcernName
    extend ActiveSupport::Concern

    included do
      # your class level code
    end

    # move your instance methods here

    module ClassMethods
      # your class methods, removing self.
    end
  end
  • Move class level code to included block.
  • Move class methods to inside of module ClassMethods.
  • Move instance methods to module.
  • Place include statement in original model class:
  include ModelName::ConcernName

Another Concern Example

  • Break out methods concerning a user's feed.
  • Now the User is broken up into:
    • user/find_methods.rb
    • user/feed_methods.rb
  • Tests similarly matched up.
  • I like the smaller files!
  • Safe, easy way to slice up huge model files.
  • Could be first step to further refactoring.

Concerns: Didn't Show You But Useful!

  • Sharing a concern in multiple models
  • Nesting concerns (concerns loading concerns)
  • Applies to controllers as well as models

Concerns Summary

Applicable Situation

  • A giant model file with corresponding giant spec file.
  • Duplicated code in multiple models.

Solution

The giant model can safely and easily be broken down into logical chunks (concerns), each with a
correspondingly smaller spec file per 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.
  4. Can DRY up code when a concern is shared among multiple models.

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 API of the model.
  2. For a complex operation involving many different methods, and possibly
    multiple models, a Service Object, aka PORO, better groups the logic.
  3. If the code view-centric, and relies on helpers, then a Decorator is a better
    choice.

Concerns References

Review on Reviewable

@gylaz
Copy link

gylaz commented Apr 8, 2014

Under "Disadvantages", points 2 and 3 are actually alternatives -- not really disadvantages. You might want to mention that another disadvantage is, it could be argued, that Concerns hide the complexity of the Object. Also, it is often times harder to find how a certain object has access to a method, and where that method is defined.

"PORO" stands for "Plain Old Ruby Object", and comes from the Java acronym of "POJO" (perhaps another language coined it earlier). Saying Service Object is also known as PORO is not really correct. PORO is just any plain ruby object, that doesn't depend on any libraries in itself.

@gylaz
Copy link

gylaz commented Apr 8, 2014

Another thing worth mentioning is trickiness involved when testing Concerns that are included into several models. Do you test each model for those included methods? Do you create a dummy model/class and test the concern once? Do you use RSpec's shared examples?

Worth to think about Concern method that rely on some state of the model. Say you have a method like this in your Concern:

def by_email
  where(email: @email).first
end

That Concern now depends on the fact that the including model must have a @email ivar (or define the method email if we go that route). That's sort of implicit API, and things like that could be missed until they blow up in production.

@dreamr
Copy link

dreamr commented Apr 13, 2014

I dislike concerns hugely. Now that I have that off my chest...

@gylaz I used to do shared spec on these kinds of things, but that was when I was 'speccing' instead of 'testing' as I do now.

When I need to test any mix-in, I create a test class, mix in the module, then test the behavior. `At least this was how I used to do it.

These days I just extend self on modules and test them directly.

module SomeShitHere
  extend self

  def a_function
    #noop
  end
end

it "blah blah" do
  SomeShitHere.method.must_equal nil
end

@justin808
Copy link
Member Author

@dreamr Thanks for the response.

  1. What's the reason for disliking concerns but OK to include modules?
  2. Please correct me, but doesn't the extend self allow you to simply call methods on the module as statics? What if the function =a_function= depends on instance values?
  3. Does the =extend self= have any purpose other than testing?

Thanks.

@dreamr
Copy link

dreamr commented Apr 13, 2014

@justin808 I write small focused, isolated classes. I also stay away from loading dependencies I don't need.

Concerns are inviting big-balls-of-shit, which I don't like. They keep you firmly in Rails-land, and they serve no purpose except to create highly coupled junk-drawers.

A much better idea would be to extract business log out into PRO (pure ruby objects). These can be modules, classes, whatever. But they should have well-defined boundaries.

Rails in general encourages highly coupled messes, which is why they always turn into a rescue at some point if they live long enough.

Off with the rant and onto one of my favorite topics, extend self.

Extend self is essentially a module-wide module_function call. It indeed turns all the 'instance' (which modules don't actually have) level stuff into class level stuff.

As I write as much stateless, immutable objects and functions as possible in my code, I end up using extend self a lot. I also create a lot more modules than classes and methods commonly delegate to a stand-alone module, ie:

module CarDiagnosticRecipeRunner
  def run(recipe)
    ...
  end
  alias_method :call, :run
end

class Car
  def run_diagnostic(recipe)
    CarDiagnosticRecipeRunner.(recipe, self)
  end
end

Whereas I think you tend to use modules like so:

module CarDiagnostics
  def run_diagnostic(recipe)
    ...
  end
end

class Car
  include CarDiagnostics
end

@justin808 justin808 changed the title Concerns Earlier Draft: Concerns Apr 19, 2014
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.

None yet

3 participants