Questioning My Design

rubyonrails-talk
1

Hi All,

I'm writing a web app for a non-profit NGO that helps farmers in a co- op plan their crops better to reduce shortages, surpluses and unhealthy competition between themselves. Right now, I'm doing something that doesn't feel Rails-ee and want some feedback. Here's the problem.

Here's the set up: the farmers are Users with a 'farmer' role. I want to all the farmer to view their account to view their User profile (name, phone, email, etc.) as well as their Sales, Purchases, and the Crops they are currently managing. (Sales and Purchases are actually both Orders handled polymorphically, which might not be important for this discussion).

In the view for User#show I have partial that displays some links that go back to the Show action but have set a param that gives context to which chunk of information the person wants to see, such as:

<ol>   <li><%= link_to 'Profile', user_path(current_user, :ui_selector => 'profile') %></li>   <li><%= link_to 'Sales', user_path(current_user, :ui_selector => 'seller') %></li>   <li><%= link_to 'Purchases', user_path(current_user, :ui_selector => 'buyer') %></li>   <li><%= link_to 'Current Crops', user_path(current_user, :ui_selector => 'crops') %></li> </ol>

You can probably tell where this question is going.... Inside the controller, User#show I set instance variables based on the value of params[:ui_selector], like thus:

@crop = Crop.new(:user_id => @user.id) if params[:ui_selector] == 'crops' @order = Order.new(:buyer_id => @user.id) if params[:ui_selector] == 'buyer' @order = Order.new(:seller_id => @user.id) if params[:ui_selector] == 'seller'

Then, also in the view User#show, I show the right stuff based on params[:ui_selector], like this:

<% if params[:ui_selector] == 'profile' %>   <p>Name: <%= @user.name %><br />     Email: <%= @user.email %><br />     Phone: <%= number_to_phone @user.cell %></p>

<% elsif params[:ui_selector] == 'seller' %>   <%= render :partial => "sales", :object => @user %>

<% elsif params[:ui_selector] == 'buyer' %>   <%= render :partial => "purchases", :object => @user %>

<% else params[:ui_selector] == 'crops' %>   <%= render :partial => "crops", :object => @user %>

<% end %>

The sales, purchases and crops partials set up a table and then basically call partials for those classes indexes using @user.sales, @user.purchases and @user.crops respectively.

It is all working okay, but alarms go off every time I use logic blocks in Rails, like there has got to be a better way, I'm missing something obvious. It doesn't feel very OOP nor DRY but I can't see what I'm missing.

Also, is it bad practice to have the views use the collections of an instance variable as I have done here with the partials accessing @user.sales, for instance? It seems like I should be setting maybe a @sales in the controller, or is the tax on the database the same? (Come to think of it, I'll probably need to move that into the controller if I want to use will_paginate with these.)

Thanks, Dee P.S. After we get this rolled out in about two-months time, I'll throw it onto github and make it open-source.

2

Hi All,

I'm writing a web app for a non-profit NGO that helps farmers in a co- op plan their crops better to reduce shortages, surpluses and unhealthy competition between themselves. Right now, I'm doing something that doesn't feel Rails-ee and want some feedback. Here's the problem.

Here's the set up: the farmers are Users with a 'farmer' role. I want to all the farmer to view their account to view their User profile (name, phone, email, etc.) as well as their Sales, Purchases, and the Crops they are currently managing. (Sales and Purchases are actually both Orders handled polymorphically, which might not be important for this discussion).

In the view for User#show I have partial that displays some links that go back to the Show action but have set a param that gives context to which chunk of information the person wants to see, such as:

Perhaps it would be easier if you had different actions/views for each:

map.resources :user, :member => { :profile => :get, :seller => :get,                                    :buyer => :get, :crops => :get }

<ol>   <li><%= link_to 'Profile', user_path(current_user, :ui_selector => 'profile') %></li>

profile_user_path(current_user)

  <li><%= link_to 'Sales', user_path(current_user, :ui_selector => 'seller') %></li>   <li><%= link_to 'Purchases', user_path(current_user, :ui_selector => 'buyer') %></li>   <li><%= link_to 'Current Crops', user_path(current_user, :ui_selector => 'crops') %></li> </ol>

You can probably tell where this question is going.... Inside the controller, User#show I set instance variables based on the value of params[:ui_selector], like thus:

@crop = Crop.new(:user_id => @user.id) if params[:ui_selector] == 'crops'

Then have a separate action for each:

def crops    @crops = @user.crops.current end

Assuming that you have an association and named_scope such as: class User    has_many :crops end class Crop    belongs_to :user    named_scope :current, { :conditions => ['season = ?', season_for(Date.today)] } end

(Of course, I'm assuming that you have a definition of what "Current Crops" are somewhere.)

Your links look like you are creating a crop, sale, purchase, or profile, but the text seems like you mean to see the current status of these things.

@order = Order.new(:buyer_id => @user.id) if params[:ui_selector] == 'buyer' @order = Order.new(:seller_id => @user.id) if params[:ui_selector] == 'seller'

Then, also in the view User#show, I show the right stuff based on params[:ui_selector], like this:

<% if params[:ui_selector] == 'profile' %>   <p>Name: <%= @user.name %><br />     Email: <%= @user.email %><br />     Phone: <%= number_to_phone @user.cell %></p>

<% elsif params[:ui_selector] == 'seller' %>   <%= render :partial => "sales", :object => @user %>

<% elsif params[:ui_selector] == 'buyer' %>   <%= render :partial => "purchases", :object => @user %>

<% else params[:ui_selector] == 'crops' %>   <%= render :partial => "crops", :object => @user %>

<% end %>

You would then have separate views for each action. You can call out to a partial if you do have common stuff (such as an order).

The sales, purchases and crops partials set up a table and then basically call partials for those classes indexes using @user.sales, @user.purchases and @user.crops respectively.

It is all working okay, but alarms go off every time I use logic blocks in Rails, like there has got to be a better way, I'm missing something obvious. It doesn't feel very OOP nor DRY but I can't see what I'm missing.

Also, is it bad practice to have the views use the collections of an instance variable as I have done here with the partials accessing @user.sales, for instance? It seems like I should be setting maybe a @sales in the controller, or is the tax on the database the same? (Come to think of it, I'll probably need to move that into the controller if I want to use will_paginate with these.)

Thanks, Dee P.S. After we get this rolled out in about two-months time, I'll throw it onto github and make it open-source.

Good Luck!

-Rob

Rob Biedenharn http://agileconsultingllc.com   Rob@AgileConsultingLLC.com

  rab@GaslightSoftware.com

3

Ah ha! That does make sense, Rob! I've had similar problems before but haven't considered additional member functions. You are correct in assuming that I have those associations and named_scopes. The "current crops" is actually a named scope as is past crops, and more collections associated with the User, which I left out for brevity.

By adding these additional members what does that do to its RESTfulness?

Thanks, Dee

4

Ah ha! That does make sense, Rob! I've had similar problems before but haven't considered additional member functions. You are correct in assuming that I have those associations and named_scopes. The "current crops" is actually a named scope as is past crops, and more collections associated with the User, which I left out for brevity.

By adding these additional members what does that do to its RESTfulness?

Thanks, Dee

Still perfectly RESTful. You'd end up with a url path like:

/user/:id/crops

Which would route to your new 'crops' action in the UserController

There's nothing magical about the typical seven actions: index, new, create, show, edit, update, destroy

You're just adding a few that return a different representation of the state of the resources related to a user.

-Rob