Rails - Dec 2010 - i created my first helper method and it works, but should this be in a helper, or controller?

I am cutting my teeth on Ruby and Rails and am trying to clean up my
views. I just created a helper method that seems to work, but wonder
if a) this should be in helper or controller  b) is the helper i
created good, bad, a step in the right direction?

The section from the original view:
<div class="even_column" style="margin-bottom:10px">
  <%= render :partial => ''job_info'' %>
  <% if @job_post.apply_method != "offline" &&
logged_in?%><%= link_to
image_tag("/images/buttons/btn_apply_now.png"),
job_apply_url(@job_post)%>
    <% end %>
  <% if @job_post.apply_method != "offline" &&
!logged_in?%><h3><span
style="color:#ff3300">Sign in to apply online for this
post.</span></
h3></br><%= link_to
image_tag("/images/buttons/btn_signin_now.png"),
login_seeker_url()%>
    <% end %>
  <%= render :partial => ''apply_options'' %>
</div>




What i moved into a helper:
module JobPostsHelper

def options_email_a
  if @job_post.apply_method != "offline" && logged_in?
  link_to image_tag("/images/buttons/btn_apply_now.png"),
job_apply_url(@job_post)
  end
end

def options_email_b
  if @job_post.apply_method != "offline" && !logged_in?
  link_to image_tag("/images/buttons/btn_signin_now.png"),
login_seeker_url()
  end
  end
end



-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en.

sol.manager wrote in post #967024:
> I am cutting my teeth on Ruby and Rails and am trying to clean up my
> views. I just created a helper method that seems to work, but wonder
> if a) this should be in helper or controller
Logic doesn''t go in the controller.
>  b) is the helper i
> created good, bad, a step in the right direction?
A little bit of each. :)

[...]
> What i moved into a helper:
> module JobPostsHelper
>
> def options_email_a
>   if @job_post.apply_method != "offline" && logged_in?
>   link_to image_tag("/images/buttons/btn_apply_now.png"),
> job_apply_url(@job_post)
>   end
> end
>
> def options_email_b
>   if @job_post.apply_method != "offline" && !logged_in?
>   link_to image_tag("/images/buttons/btn_signin_now.png"),
> login_seeker_url()
>   end
>   end
> end
This is a step in the right direction, but:
* Your method names are terrible: "a" and "b"
aren''t descriptive.  Try
to come up with nicer names.
* You can''t set an @instance variable in the controller and see it in a
helper.  Pass the JobPost as an argument to those methods.
* There''s a lot of duplication between those two methods.  You can 
probably refactor them into one, and in particular the whole complex if 
condition should be a model method.
* Indent your code properly, and don''t use empty parentheses when a 
method has no arguments.  (This isn''t Java. :) )

Good luck!

Best,
--
Marnen Laibow-Koser
http://www.marnen.org
marnen-sbuyVjPbboAdnm+yROfE0A@public.gmane.org

-- 
Posted via http://www.ruby-forum.com/.

-- 
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en.