0
\$\begingroup\$

I have the following code in a VIEW:

<select name="selected_budget_mode" class="" onchange="OnBudgetModeChanged(this)">
<option value="0"
 <% if params[:current_budget_mode].to_i == 0 %>selected
 <% end %>><%= show_budget_mode(0) %></option>
<option value="1"
 <% if params[:current_budget_mode].to_i == 1 %>selected
 <% end %>><%= show_budget_mode(1) %></option>
<option value="2"
 <% if params[:current_budget_mode].to_i == 2 %>selected
 <% end %>><%= show_budget_mode(2) %></option>
</select>

I am aware of the select_tag and the options_for_select form helpers, but that implies that I need to create a list with the options and then pass it to the options_for_select.

The budgets_mode are not dynamic. They don't come from calling a service or other function, there are only 3 modes.

What would be the best approach to simplify this code and make it as readable as possible?

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked May 7, 2015 at 14:12
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

You could still use options_for_select without much hassle:

<%= options_for_select (0..2).map { |i| [show_budget_mode(i), i] }, params[:current_budget_mode].to_i %>

Of course, I'd recommend creating the array in the controller or a helper, rather than the view. The view should ideally do as little logic as possible.

Alternatively, you could use content_tag just the keep the code cleaner:

<% (0..2).each do |i| %>
 <%= content_tag :option, show_budget_mode(i), selected: params[:current_budget_mode].to_i == i %>
<% end %>

Again, I'd recommend moving stuff to the controller. Even a current_budget_mode helper method would help clean things up:

class SomeController < ActionController
 helper_method :current_budget_mode, :budget_modes
 def current_budget_mode
 params[:current_budget_mode].to_i
 end
 def budget_modes
 (0..2).to_a
 end
end

which would clean up the view:

<% budget_modes.each do |i| %>
 <%= content_tag :option, show_budget_mode(i), selected: current_budget_mode == i %>
<% end %>

or

<%= options_for_select budget_modes.map { |i| [show_budget_mode(i), i] }, current_budget_mode %>
answered May 7, 2015 at 17:37
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.