RailsConf: Presenters #11

Open
wants to merge 3 commits into
from

Conversation

Projects
None yet
5 participants
@justin808
Member

justin808 commented Apr 18, 2014

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

Presenters

Code: Github Fat Code Refactoring Techniques Pull Request Presenters

Example: Added FollowersPresenter and FollowedPresenter to encapsulate
multiple instance variables in the show_follow view.

WARNING: This code change is an over-simplified example of a good case of when
to encapsulate the state passed to a view. As you look at this example, consider
a case where there is far more logic in the view and the controller setup of the
instance variables for the view.

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..I didn't like it, and some others gave me a
    thumbs up on the term "Presenter".

"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.

Why?

  1. You're trying to reduce the number of instance variables assigned by the
    controller action to one.
  2. You want to consolidate the view specific Ruby code in one place, rather than
    being spread inside the ERB or HAML code.

Presenter Applicable Situations

  1. Controller is initializing multiple instance variables for the view.
  2. Controller has several private methods supporting the public method
    associated with the action just to set up the context for the view.
  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

  • Create directory app/presenters and add this code to
    application.rb
  config.autoload_paths += %W(
    #{config.root}/app/presenters
  )
  • Create subdirectory app/presenters/users
  • Create presenter: User::FollowPresenter
  • Controller instantiates the User::FollowPresenter that is used by the view.
  • Move ruby code from view and the controller into the presenter.
  • Possibly include this snippet of code so that view instance methods
    are available:
        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

  1. 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.
  2. Other options for controller simplification include breaking up a controller
    into several classes and using controller concerns. However, those may not
    tackle the issue of passing too many instance variables into the view.

Review on Reviewable

justin808 added some commits Apr 18, 2014

Added Users::FollowPresenter
* Values are memoized so that queries are only done when value is needed
  for display. This helps avoid unnecessary running of queries.
* Try out application running as
  rails s -e production
* Observe queries first time and second time.
* Notice that before abstracting out the presenter, the count query in
  the controller ran even if the view was cached.
Use subclass to avoid conditional in FollowPresenter
* Created FollowPresenter, and FollowedUsersPresenter
@justin808

This comment has been minimized.

Show comment
Hide comment
@justin808

justin808 Apr 20, 2014

Member

Regarding the coding style to set at most one instance variable in a controller action, what about having using controller filters (before_action) set instance variables?

I got that comment in response to whether or my Presenters pattern (#11) is justified.

Unless one uses such before_action filters to set other instance variables, what would be other alternatives than this "presenters" pattern?

Member

justin808 commented Apr 20, 2014

Regarding the coding style to set at most one instance variable in a controller action, what about having using controller filters (before_action) set instance variables?

I got that comment in response to whether or my Presenters pattern (#11) is justified.

Unless one uses such before_action filters to set other instance variables, what would be other alternatives than this "presenters" pattern?

</section>
<section>
<%= render 'shared/stats' %>
- <% if @users.any? %>

This comment has been minimized.

@thatrubylove

thatrubylove Apr 21, 2014

tell, don't ask. you can get rid of both of these predicates on .any? by moving the logic into a helper or partial.

if the only point in these predicates are to prevent @users.each and @presenter.users.each from chunking on you, then you can do a @presenter.users.to_a.... and @users.to_a...

Then they wont blow up if there isnt anything in them as they become empty arrays!

@thatrubylove

thatrubylove Apr 21, 2014

tell, don't ask. you can get rid of both of these predicates on .any? by moving the logic into a helper or partial.

if the only point in these predicates are to prevent @users.each and @presenter.users.each from chunking on you, then you can do a @presenter.users.to_a.... and @users.to_a...

Then they wont blow up if there isnt anything in them as they become empty arrays!

- <h3><%= @subtitle %></h3>
- <% if @users.any? %>
+ <h3><%= @presenter.subtitle %></h3>
+ <% if @presenter.users.any? %>

This comment has been minimized.

@thatrubylove

thatrubylove Apr 21, 2014

so many nil checks.... :) your system is becoming quickly infested!

@thatrubylove

thatrubylove Apr 21, 2014

so many nil checks.... :) your system is becoming quickly infested!

This comment has been minimized.

@thatrubylove

thatrubylove Apr 21, 2014

nullObjectPattern or confient casting or tell don't ask - there are 3 mechanisms for ya to combat this!

@thatrubylove

thatrubylove Apr 21, 2014

nullObjectPattern or confient casting or tell don't ask - there are 3 mechanisms for ya to combat this!

@Systho

This comment has been minimized.

Show comment
Hide comment
@Systho

Systho May 17, 2014

Why do you make separate stereotypes for draper decorators and presenters ? Why not just use draper decorators as presenters and have multiple possible decorators for a given resource - a resource being an ActiveRecord, or an ActiveModel, or any kind of object which need to be manipulated by the controller.

If you have multiple draper decorators sharing behaviour, you could just use modules or concerns.

Systho commented May 17, 2014

Why do you make separate stereotypes for draper decorators and presenters ? Why not just use draper decorators as presenters and have multiple possible decorators for a given resource - a resource being an ActiveRecord, or an ActiveModel, or any kind of object which need to be manipulated by the controller.

If you have multiple draper decorators sharing behaviour, you could just use modules or concerns.

- <span><%= link_to "view my profile", @user %></span>
- <span><b>Microposts:</b> <%= @user.microposts.count %></span>
+ <%= gravatar_for @presenter.user %>
+ <h1><%= @presenter.user.name %></h1>

This comment has been minimized.

@dsibiski

dsibiski Sep 23, 2014

Does this here violate the "Law" of Demeter? The View here needs to know about the User model. It might be better to have a method named "user_name" in the presenter. This way the presenter is the only one that needs to know about the User model. The same could be said for "@presenter.user.microposts.count", although perhaps the User model (or some other object) should have a "microposts_count" method so that the presenter doesn't know about the Microposts model either.

This way, the view is no longer coupled to any other objects but the presenter. It's much easier to test, because you can easily stub out these methods, and it would be much easier to change implementation without breaking things.

Just a thought. :)

By the way, I really enjoyed your talk from RailsConf on this subject. It has given me lots to consider and try for myself. Great work!

@dsibiski

dsibiski Sep 23, 2014

Does this here violate the "Law" of Demeter? The View here needs to know about the User model. It might be better to have a method named "user_name" in the presenter. This way the presenter is the only one that needs to know about the User model. The same could be said for "@presenter.user.microposts.count", although perhaps the User model (or some other object) should have a "microposts_count" method so that the presenter doesn't know about the Microposts model either.

This way, the view is no longer coupled to any other objects but the presenter. It's much easier to test, because you can easily stub out these methods, and it would be much easier to change implementation without breaking things.

Just a thought. :)

By the way, I really enjoyed your talk from RailsConf on this subject. It has given me lots to consider and try for myself. Great work!

This comment has been minimized.

@justin808

justin808 Sep 25, 2014

Member

Hi @dsibiski, Thanks! I don't really see the value of adding a user_name method on the presenter, as it would be just a pass through. However, if there was some sort of modification to the @presenter.user.name, such as needing to to modify the value if it were too long, then that would be a fine example to justify a new method.

@justin808

justin808 Sep 25, 2014

Member

Hi @dsibiski, Thanks! I don't really see the value of adding a user_name method on the presenter, as it would be just a pass through. However, if there was some sort of modification to the @presenter.user.name, such as needing to to modify the value if it were too long, then that would be a fine example to justify a new method.

This comment has been minimized.

@justin808

justin808 Sep 25, 2014

Member

@Systho The reason I separate the concept of presenters and decorators is that I'm using the concept of a presenter as very specifically for a single view or controller action, and the decorator as something more closely related to the model that could be used on many views and controller actions. Thus, I might start with the code in the presenter and then if that logic was needed elsewhere in the program, I'd be tempted to break out the method into one on the decorator.

@justin808

justin808 Sep 25, 2014

Member

@Systho The reason I separate the concept of presenters and decorators is that I'm using the concept of a presenter as very specifically for a single view or controller action, and the decorator as something more closely related to the model that could be used on many views and controller actions. Thus, I might start with the code in the presenter and then if that logic was needed elsewhere in the program, I'd be tempted to break out the method into one on the decorator.

This comment has been minimized.

@dsibiski

dsibiski Sep 27, 2014

@justin808 You're right when you say that the method would just be a pass through, but unfortunately, without it, you are introducing some unnecessary coupling to 3rd party objects. In my opinion, that is one of the purposes of having a Presenter object. To eliminate the coupling between the View layer and the Model/data layer.

There is a portion of an article on "The Pragmatic Bookshelf" that addresses this issue exactly...under the subhead "Law of Demeter" in the article "Tell, Don't Ask" it says:

The disadvantage, of course, is that you end up writing many small wrapper methods that do very little but delegate container traversal and such. The cost tradeoff is between that inefficiency and higher class coupling.

The higher the degree of coupling between classes, the higher the odds that any change you make will break something somewhere else. This tends to create fragile, brittle code.

Depending on your application, the development and maintenance costs of high class coupling may easily swamp run-time inefficiencies in most cases.

Reference: https://pragprog.com/articles/tell-dont-ask

There is also a really great talk by Ben Orenstein that touches on these points and why they're beneficial. Check it out: https://www.youtube.com/watch?v=C0H-LyZy9Ko

You also mentioned in your previous comment:

However, if there was some sort of modification to the @presenter.user.name, such as needing to to modify the value if it were too long, then that would be a fine example to justify a new method.

I think the problem here is that when you change the model, you have to remember that you need to change things not only in the presenter, but also in the view (and perhaps in other classes - real apps are always more complex). If the presenter was the only one coupled to the Model, then this would be far less of an issue.

Of course, one could go crazy with this stuff and it might not be practical to follow this religiously for every circumstance, however, it has surely helped me to keep coupling low and save me tons of headaches especially when more features and complexity are introduced into the system.

@dsibiski

dsibiski Sep 27, 2014

@justin808 You're right when you say that the method would just be a pass through, but unfortunately, without it, you are introducing some unnecessary coupling to 3rd party objects. In my opinion, that is one of the purposes of having a Presenter object. To eliminate the coupling between the View layer and the Model/data layer.

There is a portion of an article on "The Pragmatic Bookshelf" that addresses this issue exactly...under the subhead "Law of Demeter" in the article "Tell, Don't Ask" it says:

The disadvantage, of course, is that you end up writing many small wrapper methods that do very little but delegate container traversal and such. The cost tradeoff is between that inefficiency and higher class coupling.

The higher the degree of coupling between classes, the higher the odds that any change you make will break something somewhere else. This tends to create fragile, brittle code.

Depending on your application, the development and maintenance costs of high class coupling may easily swamp run-time inefficiencies in most cases.

Reference: https://pragprog.com/articles/tell-dont-ask

There is also a really great talk by Ben Orenstein that touches on these points and why they're beneficial. Check it out: https://www.youtube.com/watch?v=C0H-LyZy9Ko

You also mentioned in your previous comment:

However, if there was some sort of modification to the @presenter.user.name, such as needing to to modify the value if it were too long, then that would be a fine example to justify a new method.

I think the problem here is that when you change the model, you have to remember that you need to change things not only in the presenter, but also in the view (and perhaps in other classes - real apps are always more complex). If the presenter was the only one coupled to the Model, then this would be far less of an issue.

Of course, one could go crazy with this stuff and it might not be practical to follow this religiously for every circumstance, however, it has surely helped me to keep coupling low and save me tons of headaches especially when more features and complexity are introduced into the system.

This comment has been minimized.

@rob-murray

rob-murray Jan 10, 2015

You can delegate specific methods to the user object. E.g. user_name would delegate the name method to the user. The key thing would be decouple the view with knowing how the what to go through to get the user name. Allows the presenter could override this and provide implementation of user_name if necessary.

@rob-murray

rob-murray Jan 10, 2015

You can delegate specific methods to the user object. E.g. user_name would delegate the name method to the user. The key thing would be decouple the view with knowing how the what to go through to get the user name. Allows the presenter could override this and provide implementation of user_name if necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment