Here’s my first code sample for my refactoring talk at RailsConf 2014:
shakacode:refactoring ← shakacode:concerns
opened 06:37PM - 31 Mar 14 UTC
# Rails Concerns
- http://api.rubyonrails.org/classes/ActiveSupport/Concern.html…
- Simplest safest refactoring
- Just cut and paste the code
- Default part of Rails
- Not just for code shared across models, but super to simply divide change a
large model file into logical chunks with associated tests.
## 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:
``` ruby
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:
``` ruby
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
- [Put chubby models on a diet with concerns](http://signalvnoise.com/posts/3372-put-chubby-models-on-a-diet-with-concerns) by @dhh
- [Reading Rails - Concern](http://monkeyandcrow.com/blog/reading_rails_concern/) by Adam Sanderson
- [Module ActiveSupport::Concern](http://api.rubyonrails.org/classes/ActiveSupport/Concern.html)
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/justin808/fat-code-refactoring-techniques/3)
I’m introducing concerns into Hartl’s Rails Tutorial web application.
Aside from lacking a surprise factor, is there any reason that I would not want to share my talk’s code samples so that I can get the most feedback possible before my talk at RailsConf?
Thanks in advance for any feedback!
daryllxd
April 2, 2014, 7:50am
2
As an alumni of the Michael Hartl book, this is great. I think that book needs a continuation. Can you add tests, too? When I was watching Destroy All Software, the fast tests (both in speed and in terms of steps per test_ are really what sold me on refactoring.
Dreamr
April 5, 2014, 5:42pm
3
I dislike concerns very much, but I wont stop that from sharing slides
It is far better to catch things before you put the slides up in HUGE MODE.
If you would like to talk about why I hate AR::Concerns we can talk. I prefer encapsulation and decoupling, and concerns violates those principles pretty handily.
I’m going to post the more advanced refactorings of going to “Decorators”, “Presenters” and “Service Objects”. Take a look at the pull requests for more details: Pull requests · shakacode/fat-code-refactoring-techniques · GitHub .
My general feeling is that Concerns (and Decorators) are a great first step toward the more advanced refactorings, and for simple operations, quite sufficient. For example, if coming on to a new project with insufficient tests, I’d be comfortable knowing I didn’t break anything by breaking a model up with a few concerns. Then I could add a test file on each of those concerns, which, yes, are really just tests on the model. However, I find smaller files useful in itself.
I’ve created pull requests for my fat controller, fat model refactoring examples on github.
I’m going to create a separate topic on Learn for each of these.
Concerns:
shakacode:refactoring ← shakacode:concerns
opened 06:37PM - 31 Mar 14 UTC
# Rails Concerns
- http://api.rubyonrails.org/classes/ActiveSupport/Concern.html…
- Simplest safest refactoring
- Just cut and paste the code
- Default part of Rails
- Not just for code shared across models, but super to simply divide change a
large model file into logical chunks with associated tests.
## 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:
``` ruby
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:
``` ruby
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
- [Put chubby models on a diet with concerns](http://signalvnoise.com/posts/3372-put-chubby-models-on-a-diet-with-concerns) by @dhh
- [Reading Rails - Concern](http://monkeyandcrow.com/blog/reading_rails_concern/) by Adam Sanderson
- [Module ActiveSupport::Concern](http://api.rubyonrails.org/classes/ActiveSupport/Concern.html)
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/justin808/fat-code-refactoring-techniques/3)
Decorators
shakacode:concerns ← shakacode:decorators
opened 02:44AM - 05 Apr 14 UTC
# Draper Decorator
- Easy to use formula for introducing decorators in your ap… plication.
- Simple to include like Concerns.
- Widely used in the Rails community.
- Works for me!
- [drapergem/draper on github](https://github.com/drapergem/draper)
- [Railscasts Draper Episode](http://railscasts.com/episodes/286-draper)
# Draper Steps
1. Add `gem 'draper'= and run =bundle`
2. Run Draper generator: `rails generate decorator User`, which creates
`app/decorators/user_decorator.rb` and
`spec/decorators/user_decorator_spec.rb`.
3. Move presentation specific view code to decorator.
1. Put `h.` in front of calls to helpers.
2. Remove references to the model in the decorator, as the method is
essentially on the model.
3. Call `model.decorate` to get a decorated instance of the model.
# Draper Applicable Situations
1. Extension to model that only applies to views and presentation.
2. Calculations done in view using model object.
3. Code in view helper that is more closely aligned with the model.
4. Code is that seems relatively general as opposed to clearly useful in only
one view.
5. If method feels relatively generic for views, but specific for a given model,
then the decorator is a good place to put the method.
# Draper Advantages
1. Relatively simple way to add functionality to the model that only applies to the
connection of the model to the view.
2. Can separate presentation specific code from the model.
3. Methods added to the model via the decorator have access to both view helper
methods as well as the model.
4. It's super easy to automatically convert the retrieved model object into it's
decorated instance, either manually or automatically, at the controller
layer.
5. Simple to find the decorator methods, such as using for verification in
feature specs.
# Draper Disadvantages
1. Shares similar problems with concerns in that the model object is still
getting fatter.
2. If there's multiple methods needed for some complex logic, that should be
broken into it's own PORO.
3. The advantage of ease of adding more methods can also be a disadvantage in
that the Decorator can turn into a junk-drawer of loosely coupled methods.
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/justin808/fat-code-refactoring-techniques/4)
Presenters:
shakacode:decorators ← shakacode:presenters
opened 03:11AM - 05 Apr 14 UTC
# A Name for a PORO
- Some Object for Complicated controller setup for a view
… - Terminology is controversial in the context of prior literature.
- Trying to describe a object that sits between a controller method and a view.
- Could have used "Facade"
# But Facade???
- Fascades just don't sound very positive to me!
- http://www.merriam-webster.com/dictionary/facade
1. the front of a building;
2. a false, superficial, or artificial appearance or effect
- OK, too negative!
# "Presenter"
- Object used to facilitate presentation of data in the view, typically
constructed in the controller method or maybe in the view.
- Smooth sounding.
- Easy to spell.
# Presenter Applicable Situation
1. Controller is initializing multiple instance variables for the view.
2. Controller has several private methods supporting the public method
associated with the action.
3. View has lots of inline Ruby code for calculating instance variables
4. Logic is specific to one controller method and one view, or maybe a couple
different response types for the same controller method.
5. Numerous view helper methods interacting just for one action.
6. If you find same code in multiple presenters for same model, then see if you
can move to a decorator or concern. A module is another option for DRY'ing up
duplication.
# Presenter Solution
1. Create directory `app/presenters` and add this code to `application.rb`
``` ruby
config.autoload_paths += %W(
#{config.root}/app/presenters
)
```
1. Create subdirectory `app/presenters/users`
2. Create presenter: `User::FollowPresenter`
3. Controller instantiates the `User::FollowPresenter` that is used by the
view.
4. Move ruby code from view and the controller into the presenter.
5. Possibly include this snippet of code so that view instance methods are
available:
``` ruby
include Draper::ViewHelpers
```
# Presenter Advantages
1. Clearly bundles the code around the work the controller is doing and what the
view needs.
2. Simplifies code at the controller and view layers.
3. Provides the possibility of unit testing the Presenter.
4. Easy to group related methods and memoize instance variables, which can be
highly beneficial if fragment caching is used, as compared to instantiating
instance variables in the controller..
5. Good place for the calculation of a complicated cache key rather than a view
helper.
# Presenter Disadvantages
There might be simpler techniques than creating another class and object, such
as putting some logic in the Draper decorator or a view helper. A few lines of
inline ruby code may work better than moving this code to a separate class.
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/justin808/fat-code-refactoring-techniques/5)
Service Objects:
shakacode:presenters ← shakacode:service_objects
opened 07:49AM - 06 Apr 14 UTC
# Service Objects
This code demonstrates moving complex controller code into a … class that manages model interactions, aka a `Service Object`. The controller calls the Service Object and it returns an object that encapsulates what the controller should do. This includes flash
messages, redirects, etc. The example class `ControllerResponse` provides a
sample of how to do this. A service object could create a presenter for the
controller to pass to the view.
After getting this reviewed, I've developed an alternative approach that focuses on keeping controller parts (setting flash, etc.) in the controller and moves other logic into the model code. See https://github.com/justin808/fat-code-refactoring-techniques/pull/12.
# Service Objects Applicable Situation
1. Some action involving the interaction between several models, such as:
- A customer creating an order and charging the purchase.
- Post login process for a user.
2. Long complicated methods, and groups of methods on a single model.
# Service Objects Example for Micro Blog
1. Minors are not allowed to post profanity.
2. User model counts the number of profanity words used by a minor.
3. First commit presents a tangled up controller method:
``` ruby
def create
@micropost = current_user.microposts.build(micropost_params)
ok = true
if current_user.minor?
profanity_word = profanity_word(@micropost.content)
if profanity_word.present?
flash.now[:error] = "You cannot create a micropost with profanity: '#{profanity_word}'!"
current_user.increment(:profanity_count)
@feed_items = []
render 'static_pages/home'
ok = false
end
end
if ok
if @micropost.save
flash[:success] = "Micropost created!"
redirect_to root_url
else
@feed_items = []
render 'static_pages/home'
end
end
end
```
This code is not easily testable, being on the controller. The addition of the
`profanity_words` method on the controller also does not fit. I don't go into
building tests, as the refactor will allow easy testing.
``` ruby
PROFANITY_WORDS = %w(poop fart fartface poopface poopbuttface)
# Yes, this could go into a validator for the micropost, but let's suppose there's reasons
# that we don't want to do that, such as we only want to filter profanity for posts
# created by minors, etc.
# returns profanity word if existing in content, or else nil
def profanity_word(content)
words = content.split(/\W/)
PROFANITY_WORDS.each do |test_word|
puts "test_word is #{test_word}, words is #{words}"
return test_word if words.include?(test_word)
end
nil
end
```
4. After adding MicropostCreationService:
1. The code is much easier to test.
2. The controller method is much simpler:
``` ruby
def create
response = MicropostCreationService.new(current_user, micropost_params[:content]).create_micropost
response.apply(self)
unless response.ok # if not ok, then we did a redirect when calling response.apply
@micropost = response.data[:micropost]
@feed_items = []
render 'static_pages/home'
end
end
```
The `response.apply` method does things like setting the flash message and
the response code.
3. The main method of `MicropostCreationService` is simple:
``` ruby
def create_micropost
micropost = @user.microposts.build(content: @content)
response = check_profanity(micropost)
unless response
response = save_micropost(micropost)
end
response
end
```
4. Note how the Service utilizes the `ControllerResponse` class so as not be
throwing exceptions for error scenarios.
``` ruby
def save_micropost(micropost)
if micropost.save
ControllerResponse.new(flash_message: "Micropost created!",
flash_type: :success,
redirect_path: h.root_url)
else
ControllerResponse.new(http_status: :bad_request, data: { micropost: micropost })
end
end
# return response or
def check_profanity(micropost)
if profanity_word
msg = "Whoa, better watch your language! Profanity: '#{profanity_word}' not allowed!"
@user.increment(:profanity_count)
ControllerResponse.new(flash_message: msg,
flash_type: :error,
flash_now: true, # True b/c not redirecting
http_status: :bad_request,
data: { micropost: micropost })
end
end
```
5. A good test of code is how it changes when requirements change. Suppose that
the code should print all the profanity in the message, and add the number of
profanity words to the user's profanity counter.
6. Putting the profanity calculation into it's own class further shrinks the
size of classes and clarifies the tests. The main advantage of having the
`ProfanityChecker` into its own class is that the Micropost can also use the
logic for validation.
7. A slight error was left in the commits along with the fix. This is to show
the usefulness of having concise and simple unit tests on the
`MicropostCreationService`.
# Service Object Solution
1. Create a PORO in a `services` directory (or optionally in models).
2. If using the services directory, be sure to add it to the load path
``` ruby
config.autoload_paths += %W(
#{config.root}/app/services
)
```
1. Move all applicable methods into this class from the model.
2. Use an instance of ControllerResponse to return the responses back to the
controller.
3. Create test in `spec/services` directory.
# Service Object Advantages
1. Clearly bundles code around a specific, complex action.
2. Promotes the creation of smaller, tighter methods when all methods in a small
class are around a single purpose.
3. Allows better unit testing of methods on this service object.
# Service Object Disadvantages
- It can be overkill for a very simple action.
- Something simply and generally useful on the model is better put in a concern
or decorator, or maybe a method class called by the model.
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/justin808/fat-code-refactoring-techniques/6)
zamith
April 7, 2014, 8:14am
6
@Justin_Gordon Good job. Hope the talk goes well.
My problem with concerns is that they are mixins, which in turn are ruby’s way of having multiple inheritance. This raises an alarm for me. Not that inheritance is bad, it is not, it is a very powerful technique and concept, but it is often misused and overused I dare say.
I believe the first approach should be composition and only after trying that do I go for other solutions. Adding a concerns directory is something I do not agree with, because I think it encourages the wrong things. Even ActiveSupport::Concern has it’s share of problems .
That being said, it is a legitimate solution to try as long as one is aware that it is a form of inheritance and all of the pros and cons that has.
1 Like
I agree with @zamith on this. Also, I’ve come to noticed that Concerns are getting used as a macro to avoid typing… and thus further hide an underlying problem.
@Justin_Gordon , hope the talk goes well!
