Refactoring best practices

Hi all, I’ve been developing a Rails app, I almost finish so, I decided refactor my code. Here some doubts

1.- I’ve Job and User controllers, a user has jobs but when you create the job there isn’t user assigned.

So I’ve the myworks in the entries controller:

def myworks @entries = Entry.find_all_by_user_id(current_user.id) respond_to do |format|

  format.html
  format.xml  { render :xml => @entries }
end

end

and I’ve this route:

match ‘/myworks’ => ‘entries#myworks’, :as => ‘myworks’

So I access it via the URL: localhost:3000/myworks

But I move it to the User controller (I think it’s better):

def myworks @entries = Entry.find_all_by_user_id(self.id)

respond_to do |format|
  format.html
  format.xml  { render :xml => @entries }
end

end

and I want to access It with this url: localhost:3000/user/1/myentries

Question: How do I to do that?

2.- I’ve this img tag many times in my views:

<img src=“<%= entry.officeentrystate ? “…/images/buttons/flag_green.gif” : “…/images/buttons/flag_red.gif”%>” title=“<%= t(‘entries.fields.officeentrystate’) %>” alt=“<%= t(‘entries.fields.officeentrystate’) %>”/>

Question: There’s a way to create a helper to do that? Like,

helper_name(title, value)

I’ll really appreciate your help.

Regards,

Amador

I think you need to nest the models in the routes.rb file This will give you the url's you require

  map.resources :users, :has_many => :entries

This give the following routes: (run "rake routes | less" to see full routes list and link_to path and url's)

Check out http://railscasts.com episode 70 "Custom routes", 34 "Named routes" and then go on to 139 "nested resources"

Send the models you have so we can see the relationships

OK:

My User model:

require ‘digest’ class User < ActiveRecord::Base attr_accessor :passwd validates :passwd, :confirmation => true, :length => { :within => 6…20 },

:presence => true,
:if => :password_required?

validates :nickname, :uniqueness => true, :length => { :within => 5…50} has_many :comments has_many :entries, :class_name => “User”, :foreign_key => “geomatics_elaboration_id”

has_and_belongs_to_many :entries before_save :encrypt_new_password def fullname (self.firstname + " " + self.lastname) end def self.authenticate(nick, passwd) user = User.find_by_nickname(nick)

return user if (user and user.active and user.authenticated?(passwd))

end def authenticated?(passwd) self.password == encrypt(passwd) end protected def encrypt_new_password return if passwd.blank?

self.password = encrypt(passwd)

end def password_required? password.blank? or passwd.present? end def encrypt(string) Digest::SHA1.hexdigest(string) end end My Entry model:

Hi all, I’ve been developing a Rails app, I almost finish so, I decided refactor my code. Here some doubts

1.- I’ve Job and User controllers, a user has jobs but when you create the job there isn’t user assigned.

So I’ve the myworks in the entries controller:

def myworks @entries = Entry.find_all_by_user_id(current_user.id)

respond_to do |format|

  format.html
  format.xml  { render :xml => @entries }
end

end

and I’ve this route:

match ‘/myworks’ => ‘entries#myworks’, :as => ‘myworks’

So I access it via the URL: localhost:3000/myworks

But I move it to the User controller (I think it’s better):

def myworks @entries = Entry.find_all_by_user_id(self.id)

respond_to do |format|
  format.html
  format.xml  { render :xml => @entries }
end

end

and I want to access It with this url: localhost:3000/user/1/myentries

Question: How do I to do that?

Yes, It makes sense to put that method in the User controller. For the routes, you could do something like

match ‘/user/:id/myentries’ => ‘entries#myworks’

and then in your method, change self.id to params[:id] in the line @entries = Entry.find_all_by_user_id([ params:id] )

2.- I’ve this img tag many times in my views:

<img src=“<%= entry.officeentrystate ? “…/images/buttons/flag_green.gif” : “…/images/buttons/flag_red.gif”%>” title=“<%= t(‘entries.fields.officeentrystate’) %>” alt=“<%= t(‘entries.fields.officeentrystate’) %>”/>

Question: There’s a way to create a helper to do that? Like,

helper_name(title, value)

I guess, you are passing the entry object to the above image tag, make a helper call it whatever you want

def helper_name entry <img src=“<%= entry.officeentrystate ? “…/images/buttons/flag_green.gif” : “…/images/buttons/flag_red.gif”%>”

title=“<%= t(‘entries.fields.officeentrystate’) %>” alt=“<%= t(‘entries.fields.officeentrystate’) %>”/> … … end

and then call the helper in the view by <%= helper_name entry %>

If you want to use the helper in many views across many models, write the helper in the application_helper.rb file.

Let me know if anything doesn’t work out. Happy to help.