Topic: really ugly code to refactor

all this is in my view

<% if current_user.couple.nil? %>
  <%= render 'couple' %> <!-- Form to search for user -->
<% else %>
  <% if current_user.couple %>
    Waiting for  <%= current_user.couple.profile %> to accept.
    <%= link_to 'Undo', couple_path(current_user), :confirm => "Are you sure?" , :method => :delete %>
  <% else %>
    <% user = User.find(current_user.couple.couple_id) %>
    <% unless user.couple.nil? && (user.couple.couple_id == current_user) %>
      Something
    <% end %>
  <% end %>
<% end %>

I know is not correct to have all this in my view, havng in mind that 'Something' means more code

Last edited by jtomasrl (2011-10-23 11:48:40)

Re: really ugly code to refactor

Depends on what 'something is',  but normally the way to clean up views is with Helpers,  see if you can re-create whatever 'something' is in a Helper.

Joe got a job, on the day shift, at the Utility Muffin Research Kitchen, arrogantly twisting the sterile canvas snout of a fully charged icing anointment utensil.

Re: really ugly code to refactor

well, partials do the job and deleted some duplicated code

<% if current_user.couple.nil? %>
  <%= render 'user_search' %> <!-- Form to search for user -->
<% else %>
  <% user = User.find(current_user.couple.couple_id) %>
  <% unless user.couple && (user.couple.couple_id == current_user.id) %>
   Waiting for <%= user.profile %> to accept.
    <%= link_to 'Undo', couple_path(current_user), :confirm => "Are you sure?" , :method => :delete %>
  <% else %>
    <%= render 'couple' %>
  <% end %>
<% end %>

Re: really ugly code to refactor

...
<% user = User.find(current_user.couple.couple_id) %>
...

I think that this really should go into controller wink.

Re: really ugly code to refactor

I think this tutorial would be perfect for you.

http://railscasts.com/episodes/286-draper?autoplay=true

Last edited by Kayle (2011-10-24 14:04:44)

Re: really ugly code to refactor

current_user.couple.couple_id
How could it be if user couple is false( if current_user.couple => else)?

What the difference between (if current_user.couple.nil?) and (if current_user.couple)? Do you want to treat different with database inconsistency and empty field?

user.couple.nil? && (user.couple.couple_id == current_user
I think it would be better if you create method in User model instead of it:
def is_couple(user)
self.couple.try(:couple_id) == user
end

And also I think it would be better to create @couple instance in controller to not request database each time
@couple = current_user.coulpe

I don't know your domain logic, but I think here is too much conditions.