Merge pull request #13347 from jeremy/concerning

Introduce Module#concerning

activesupport/CHANGELOG.md

@@ -1,3 +1,50 @@
+*   Introduce Module#concerning: a natural, low-ceremony way to separate
+    responsibilities within a class.
+
+    Imported from https://github.com/37signals/concerning#readme
+
+      class Todo < ActiveRecord::Base
+        concerning :EventTracking do
+          included do
+            has_many :events
+          end
+
+          def latest_event
+            ...
+          end
+
+          private
+            def some_internal_method
+              ...
+            end
+        end
+
+        concerning :Trashable do
+          def trashed?
+            ...
+          end
+
+          def latest_event
+            super some_option: true
+          end
+        end
+      end
+
+    is equivalent to defining these modules inline, extending them into
+    concerns, then mixing them in to the class.
+
+    Inline concerns tame "junk drawer" classes that intersperse many unrelated
+    class-level declarations, public instance methods, and private
+    implementation. Coalesce related bits and give them definition.
+    These are a stepping stone toward future growth & refactoring.
+
+    When to move on from an inline concern:
+     * Encapsulating state? Extract collaborator object.
+     * Encompassing more public behavior or implementation? Move to separate file.
+     * Sharing behavior among classes? Move to separate file.
+
+    *Jeremy Kemper*
+
 *   Fix file descriptor being leaked on each call to `Kernel.silence_stream`.
 
     *Mario Visic*

activesupport/lib/active_support/core_ext/module.rb

@@ -4,6 +4,7 @@
 require 'active_support/core_ext/module/reachable'
 require 'active_support/core_ext/module/attribute_accessors'
 require 'active_support/core_ext/module/attr_internal'
+require 'active_support/core_ext/module/concerning'
 require 'active_support/core_ext/module/delegation'
 require 'active_support/core_ext/module/deprecation'
 require 'active_support/core_ext/module/remove_method'

activesupport/lib/active_support/core_ext/module/concerning.rb

@@ -0,0 +1,136 @@
+require 'active_support/concern'
+
+class Module
+  # = Bite-sized separation of concerns
+  #
+  # We often find ourselves with a medium-sized chunk of behavior that we'd
+  # like to extract, but only mix in to a single class.
+  #
+  # Extracting a plain old Ruby object to encapsulate it and collaborate or
+  # delegate to the original object is often a good choice, but when there's
+  # no additional state to encapsulate or we're making DSL-style declarations
+  # about the parent class, introducing new collaborators can obfuscate rather
+  # than simplify.
+  #
+  # The typical route is to just dump everything in a monolithic class, perhaps
+  # with a comment, as a least-bad alternative. Using modules in separate files
+  # means tedious sifting to get a big-picture view.
+  #
+  # = Dissatisfying ways to separate small concerns
+  #
+  # == Using comments:
+  #
+  #   class Todo
+  #     # Other todo implementation
+  #     # ...
+  #
+  #     ## Event tracking
+  #     has_many :events
+  #
+  #     before_create :track_creation
+  #     after_destroy :track_deletion
+  #
+  #     private
+  #       def track_creation
+  #         # ...
+  #       end
+  #   end
+  #
+  # == With an inline module:
+  #
+  # Noisy syntax.
+  #
+  #   class Todo
+  #     # Other todo implementation
+  #     # ...
+  #
+  #     module EventTracking
+  #       extend ActiveSupport::Concern
+  #
+  #       included do
+  #         has_many :events
+  #         before_create :track_creation
+  #         after_destroy :track_deletion
+  #       end
+  #
+  #       private
+  #         def track_creation
+  #           # ...
+  #         end
+  #     end
+  #     include EventTracking
+  #   end
+  #
+  # == Mix-in noise exiled to its own file:
+  #
+  # Once our chunk of behavior starts pushing the scroll-to-understand it
+  # boundary, we give in and move it to a separate file. At this size, the
+  # overhead feels in good proportion to the size of our extraction, despite
+  # diluting our at-a-glance sense of how things really work.
+  #
+  #   class Todo
+  #     # Other todo implementation
+  #     # ...
+  #
+  #     include TodoEventTracking
+  #   end
+  #
+  # = Introducing Module#concerning
+  #
+  # By quieting the mix-in noise, we arrive at a natural, low-ceremony way to
+  # separate bite-sized concerns.
+  #
+  #   class Todo
+  #     # Other todo implementation
+  #     # ...
+  #
+  #     concerning :EventTracking do
+  #       included do
+  #         has_many :events
+  #         before_create :track_creation
+  #         after_destroy :track_deletion
+  #       end
+  #
+  #       private
+  #         def track_creation
+  #           # ...
+  #         end
+  #     end
+  #   end
+  #
+  #   Todo.ancestors
+  #   # => Todo, Todo::EventTracking, Object
+  #
+  # This small step has some wonderful ripple effects. We can
+  # * grok the behavior of our class in one glance,
+  # * clean up monolithic junk-drawer classes by separating their concerns, and
+  # * stop leaning on protected/private for crude "this is internal stuff" modularity.
+  module Concerning
+    # Define a new concern and mix it in.
+    def concerning(topic, &block)
+      include concern(topic, &block)
+    end
+
+    # A low-cruft shortcut to define a concern.
+    #
+    #   concern :EventTracking do
+    #     ...
+    #   end
+    #
+    # is equivalent to
+    #
+    #   module EventTracking
+    #     extend ActiveSupport::Concern
+    #
+    #     ...
+    #   end
+    #   include EventTracking
+    def concern(topic, &module_definition)
+      const_set topic, Module.new {
+        extend ::ActiveSupport::Concern
+        module_eval(&module_definition)
+      }
+    end
+  end
+  include Concerning
+end

activesupport/test/core_ext/module/concerning_test.rb

@@ -0,0 +1,35 @@
+require 'abstract_unit'
+require 'active_support/core_ext/module/concerning'
+
+class ConcerningTest < ActiveSupport::TestCase
+  def test_concern_shortcut_creates_a_module_but_doesnt_include_it
+    mod = Module.new { concern(:Foo) { } }
+    assert_kind_of Module, mod::Foo
+    assert mod::Foo.respond_to?(:included)
+    assert !mod.ancestors.include?(mod::Foo), mod.ancestors.inspect
+  end
+
+  def test_concern_creates_a_module_extended_with_active_support_concern
+    klass = Class.new do
+      concern :Foo do
+        included { @foo = 1 }
+        def should_be_public; end
+      end
+    end
+
+    # Declares a concern but doesn't include it
+    assert_kind_of Module, klass::Foo
+    assert !klass.ancestors.include?(klass::Foo), klass.ancestors.inspect
+
+    # Public method visibility by default
+    assert klass::Foo.public_instance_methods.map(&:to_s).include?('should_be_public')
+
+    # Calls included hook
+    assert_equal 1, Class.new { include klass::Foo }.instance_variable_get('@foo')
+  end
+
+  def test_concerning_declares_a_concern_and_includes_it_immediately
+    klass = Class.new { concerning(:Foo) { } }
+    assert klass.ancestors.include?(klass::Foo), klass.ancestors.inspect
+  end
+end