2
\$\begingroup\$

A Rails 4.2 app has three models:

  • User
  • Author
  • Quote

... and the following associations:

  • users HABTM authors
  • author has_many quotes

If the user is associated with any authors, his "profile" view should display statistics about the number of quotes of each author.

Example:

Your author(s) Mark Twain has 73 quotes, Charles Dickens has 92 quotes, and Oscar Wilde has 53 quotes.

If the user is not associated with any authors, that same view should display a different message.

Within the user model (app/models/user.rb):

def authors_statistics
  statistics = Array.new
  self.authors.each do |author|
    statistics << "#{author.name} has #{author.quotes.count} quotes"
  end
  statistics.to_sentence
end

Within the user profile view (ERB):

<% if current_user.authors_statistics.empty? %>
    You have not yet been given access to edit any authors&#39; quotes. Please
    <%= link_to 'send us a message', new_contact_form_path %> specifying to
    whose authors quotes you would like to contribute.
<% else %>
    Your author(s) <%= current_user.authors_statistics %>
<% end.to_sentence %>.

It seems to me that there is excessive coupling between the model and the view. Can you think of a better way to do it?

\$\endgroup\$

1 Answer 1

Reset to default
3
\$\begingroup\$

I'd add a counter cache to Author, i.e. quote_count just to avoid doing the extra query to fetch the number of quotes.

Don't put view concerns (the sentence, in this case) in a model; use I18n/localization instead, especially since you're dealing with pluralization (and right now you're taking the lazy route of saying author(s) and assuming quotes, plural).

Even if there's only English, the I18n stuff can do some tricks, for instance (in config/locales/en.yml or similar):

en:
  users:
    show:
      your_authors:
        one: "Your author"
        other: "Your authors"
      author_quotes:
        one: "%{name} has %{count} quote"
        other: "%{name} has %{count} quotes"

With that your view can be something like:

<%= t(".your_authors", count: @authors.count) %>
<%= @authors.map { |author| t(".author_quotes", name: author.name, count: author.quotes.size }.to_sentence %>

And #author_statistics can be removed entirely.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ Wow, a :counter_cache, I was going to be tempted to do something similar. \$\endgroup\$
    – Marc-Andre
    Commented Jan 19, 2017 at 20:24
  • 1
    \$\begingroup\$ I'll definitely implement a counter cache in this and other models too. About generating the entire sentences at the view, I had thought about it, but after reading many times that we should avoid placing logic in the view, I attempted to place as much logic as possible in the model. However, looking at it from the perspective of separation of concerns, it makes sense to place it in the view. The application is already internationalized with i18n, so your suggestion should fit right in. Your reply was very helpful, thank you! \$\endgroup\$
    – BrunoF
    Commented Jan 20, 2017 at 12:27
  • 2
    \$\begingroup\$ @BrunoFacca Glad to help. And it's true: Avoid logic in the view! However, I'd argue that calling to_sentence is really more about presentation which makes it a view-concern. It's sort of in the same vein as "humanizing" a timestamp to say "1 minute ago", or rounding a float for display. But calling map is a not quite as nice - it'd be good to move that to a UserHelper method, actually \$\endgroup\$
    – Flambino
    Commented Jan 20, 2017 at 12:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.