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

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,