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

PoC: Presenters [WIP] #21

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

mdesantis
Copy link

@mdesantis mdesantis commented Jan 14, 2019

PoC: Presenters [WIP]

Introduction

Hello, I'm trying to find a better way to handle view logic on Solidus.

Brief recap about view logic management patterns

Helpers have always been the Achilles' heel in the full object oriented world of Rails. Even Steve Klabnik complained about them. So, what are the alternatives?

Presenter pattern

The Presenter pattern couples view logic with the controller / controller and action. So, for example, if you have a PagesController with home action, you could write:

class PagesPresenter < ApplicationPresenter
  def nav_items
    [ { name: 'Home', path: home_path } ,
      { name: 'About', path: about_path } ]
  end

  def greeting
    "Hi, #{current_user.nickname}!" if current_user
  end
end

class Pages::HomePresenter < PagesPresenter
  def recent_blog_posts
    Post.order(published_at: :asc).limit(10)
  end
end

Presentation Model (aka Decorator) pattern

The Decorator pattern couples view logic with the model instance. So, for example, if you have a User model, you could write:

class UserDecorator
  def full_name
    "#{first_name} #{last_name}"
  end
end

UserDecorator.decorate(User.new)

In this PoC I'm focusing on Decorator pattern, mainly because, looking into Solidus helpers, they seem to me to a greater extent reated to models.

Alternatives

First, I looked into the current state of things regarding existing Ruby presenters implementations. This is the summary:

Draper

API samples

Article.find(params[:id]).decorate

pros

  • full featured

  • has been around for a while

  • pretty popular (~ 4.5k GH stars)

cons

  • a bit bloated

  • maintenance is "meh"

  • Decorator suffix can't be customized (and we already have decorators concept which would lead to confusion)

ActiveDecorator

API samples

# inside views, controllers or helpers
@author.full_name

pros

  • from Kaminari's author

  • simple and clean

  • Decorator suffix can be customized

cons

  • not very popular (~ 200 GH stars)

  • has auto-decoration. To me it means that we could have many issues like "why is country.name in the controller/view different from self.name in the model class?!"

  • heavy use of monkey patching

MetaPresenter

pros

  • Presenter pattern

  • Uses Presenter instead of Decorator

cons

  • Presenter pattern

  • still in early stage

  • few GH stars (5)

GitLab Presenters

API samples

@project.present(current_user: current_user)

pros

  • Simple and effective

  • Doesn't include all helpers, just URL helpers (URLs are related more to model presentation than to view specific logic)

  • We can take the best from it and customize to our needs

cons

  • Being not a third party gem, we need to design the API and maintain the code

Proposal

My proposal is based on GitLab Presenters.

API samples

Being this a proof of concept, the implemented API is very simple, its only implemented feature is presentee instance method delegation (see Further features section below for more):

module Spree
  class CountryPresenter < Spree::Presenter::Base
    def name
      "Presented name: #{super}"
    end
  end
end

italy = Spree::Country.new(name: 'Italy')
Spree::CountryPresenter.new(italy).name #=> "Presented name: Italy"
Spree::CountryPresenter.new(italy).presentee == italy #=> true

Helper methods / view context

IMHO, we shouldn't include any helpers / view context by default. Presenters are model formatters, they shouldn't know anything about the view format: that's a render responsibility. Mixing view format specific logic to presenters leads to spaghetti monsters like this one. I would include only routing and authorization helpers by default. Also, consider that to_partial_path provides much flexibility about chosing the right view.

If you really need an helper method, you can include the helper class you need, or pass the view context from the view as argument to the presenter instance.

Performance

Unfortunately, more abstraction comes with runtime overhead cost (ActiveDecorator branch here):

# Helpers only
include Spree::BaseHelper; countries = Spree::Country.all; puts Benchmark.measure { 100.times { countries_for_select countries } }
#  0.921976   0.036577   0.958553 (  0.953708)

# CountryListPresenter + CountryPresenter
countries = Spree::Country.all; puts Benchmark.measure { 100.times { Spree::CountryListPresenter.new(countries) } }
#  2.680967   0.019594   2.700561 (  2.703916)

# CountryActiveDecoratorPresenter
countries = Spree::Country.all; puts Benchmark.measure { 100.times { ActiveDecorator::Decorator.instance.decorate(countries).sort_by { |c| c.name.parameterize } } }
#  3.500285   0.035593   3.535878 (  3.530037)

I could verify that performance degradation is mainly due to presenter instances allocation, so it can't be reduced (object oriented programming is the main advantage of presenters). So, performance must be taken in consideration during collection presenters development.

Further features (to be discussed)

  • Syntactic sugar in order to create presenter instances from object instances. API sample:

    class Country
      include Presentable
    end
    
    Country.new.present

    IMHO: yes

  • passing attributes as arguments. API sample:

    class Spree::OrderPresenter < Spree::Presenter::Base
      def default_view
        current_user.try(:admin?) ? 'admin_dashboard' : 'my_orders'
      end
    end
    
    Spree::OrderPresenter.new(@order).present(locals: { current_user: current_user }).default_view

    IMHO: yes

  • Passing custom presenter to present method. Sample API:

    class Spree::Adjustment::StateIconPresenter < Spree::Presenter::Base
      def state_to_font_awesome_class
        presentee.finalized? ? 'fa-lock' : 'fa-unlock'
      end
    
      def to_partial_path
        'spree/adjustments/state'
      end
    end
    
    # spree/adjustments/_state.html.erb
    # <%= content_tag(:span, '', class: "fa fa-#{adjustment.state_to_font_awesome_class}") %>
    
    presented_object = adjustment.present presenter: 'Spree::Adjustment::StateIcon'
    render presented_object, as: :adjustment

    IMHO: yes. Useful in conjuction with render, even more with a presents helper.

  • present helper, as syntactic sugar for render object.present. API sample:

    present adjustment
    # Syntactic sugar for:
    render adjustment.present
    
    present adjustment, presenter: 'Spree::Adjustment::StateIcon', as: :adjustment
    # Syntactic sugar for:
    presenter = adjustment.present as: 'Spree::Adjustment::StateIcon'
    render presenter, as: :adjustment # inferred using presentee.class.underscore

    IMHO: yes, I think it's useful to keep things DRY.

  • Extending helper methods by default. API sample:

    class Spree::AdjustmentPresenter < Spree::Presenter::Base
      def state_icon
        icon = presentee.finalized? ? 'lock' : 'unlock'
        content_tag(:span, '', class: "fa fa-#{icon}")
      end
    end

    IMHO: no. render adjustment.present as: 'Spree::Adjustment::StateIcon' could be used instead.

  • Implicit view_context method. API sample:

    class Spree::UserPresenter < Spree::Presenter::Base
      def my_profile?
        view_context.current_user == presentee
      end
    end
    
    User.all.each do |user|
      Spree::UserPresenter.new(user).present
    end

    IMHO: no. Hard to be properly implemented. Best implementation I've seen (ActiveDecorator) requires Rails monkeypatching. Needed locals can be passed to the presenter as arguments. Needed helpers: see above.

@mdesantis mdesantis changed the title Poc presenters [WIP] PoC: presenters [WIP] Jan 14, 2019
@mdesantis mdesantis changed the title PoC: presenters [WIP] PoC: Presenters [WIP] Jan 14, 2019
@mdesantis mdesantis force-pushed the poc-presenters branch 3 times, most recently from 442773b to c03dd8c Compare January 15, 2019 11:14
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.

1 participant