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?
1 Answer 1
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 %>