I created my first helper method and it works, but should this be in a helper, or controller?

Rails
1

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:

<%= 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?%>

Sign in to apply online for this post.<%= link_to image_tag("/images/buttons/btn_signin_now.png"), login_seeker_url()%> <% end %> <%= render :partial => 'apply_options' %>

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

2

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. :slight_smile:

[…]

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. :slight_smile: )

Good luck!

Best,

Marnen Laibow-Koser
http://www.marnen.org
[email protected]