Help me clean up UGLY code

apswartz · April 30, 2008, 9:45pm

In my controller, I have the following:
def method1
displayList(:method1)
end

def method2
displayList(:method2)
end

def method3
displayList(:method3)
end

def method4
displayList(:method4)
end

def method5
displayList(:method5)
end

This just strikes me as ugly…

Any suggestions on how to clean this up? (obviously I changed the
names to make it a more generic issue.)

Thanks,

Alan

apswartz · April 30, 2008, 9:54pm

%(method1 method2 method3).each do |m|
define_method(m) do
displayList(m)
end
end

should do it. Or if it has to be a symbol, you can do

displayList(:"#{m}")

Jason

apswartz · April 30, 2008, 10:35pm

Please excuse my ignorance, but where in the controller do you put this
code?

–Alan

apswartz · April 30, 2008, 10:51pm

Anywhere outside of a method. It’s run when the class is loaded. Just
simple case of Ruby’s meta programming.

Jason

apswartz · April 30, 2008, 10:53pm

Jason has kindly provided the “how” to do this, but I would question
they
“why” you are doing this?
What’s the need to have a bunch of methods that do nothing other than
routing to another method? Maybe there should be one dispatching
methods
that calls the others?

just a thought,
/Shawn

On Wed, Apr 30, 2008 at 1:50 PM, Jason R. [email protected]

apswartz · April 30, 2008, 11:24pm

What I am trying to do is have multiple ways of looking at “slices” of
the data.

For example:
site/inbox -> Shows only items in the “inbox” state
site/state1 -> Shows items in the “state1” state
site/state2 -> Shows items in the “state2” state

Would you recommend another way of doing this?

–Alan

apswartz · April 30, 2008, 11:32pm

On Wed, Apr 30, 2008 at 2:24 PM, Alan S. [email protected] wrote:

For example:
site/inbox → Shows only items in the “inbox” state
site/state1 → Shows items in the “state1” state
site/state2 → Shows items in the “state2” state

Would you recommend another way of doing this?

I don’t know if this would be “recommended” – I’m new here too –
but you could certainly dry it up easily with ‘method_missing’.

FWIW,

Hassan S. ------------------------ [email protected]

apswartz · April 30, 2008, 11:34pm

On 30 Apr 2008, at 22:31, Hassan S. wrote:

I don’t know if this would be “recommended” – I’m new here too –
but you could certainly dry it up easily with ‘method_missing’.

why not have a show state method which takes as a parameter the state
to show ?

Fred

apswartz · May 1, 2008, 12:51am

On Apr 30, 2008, at 5:33 PM, Frederick C. wrote:

I don’t know if this would be “recommended” – I’m new here too

but you could certainly dry it up easily with ‘method_missing’.

why not have a show state method which takes as a parameter the state
to show ?

Fred

create a new route that makes that bit be a parameter:

in config/routes.rb:
map.connect ‘site/:state’, :controller => ‘alan’, :action
=> :displayList

class AlanController
def displayList
@items = Item.find(:all, :conditions => { :state =>
params[:state] })
end
end

If you can make a generic example, I can respond with generic code,
right?

-Rob

Rob B. http://agileconsultingllc.com
[email protected]

apswartz · May 1, 2008, 3:35am

Rob,
That looks pretty close. I definitely do NOT understand the routing
magic well enough. I guess I put this later so that other areas like
site/login and site/dashboard, would not be interpreted under this.

Thanks,

Alan

On Wed, Apr 30, 2008 at 5:50 PM, Rob B.

apswartz · May 1, 2008, 9:21pm

Thanks Rob,
That was more of what I was trying to get across. that the states
themselves are/should be parameters.

/Shawn

apswartz · May 1, 2008, 10:03pm

Here is how I ended up solving it (thanks to everyone)
In routes.rb
map.inbox ‘/inbox’, :controller => ‘tasks’, :action => ‘task_list’, :state =>‘inbox’ map.actions ‘/actions’, :controller => ‘tasks’, :action => ‘task_list’, :state =>‘actions’ map.waiting ‘/waiting’, :controller => ‘tasks’, :action => ‘task_list’, :state =>‘waiting’ map.someday ‘/someday’, :controller => ‘tasks’, :action => ‘task_list’, :state =>‘someday’ map.ticklers ‘/ticklers’, :controller => ‘tasks’, :action => ‘task_list’, :state =>‘ticklers’ map.reference ‘/reference’, :controller => ‘tasks’, :action => ‘task_list’, :state =>‘reference’

In tasks_controller.rb, there is a task_list method

Thanks again to everyone for your help!!

–Alan

apswartz · May 1, 2008, 10:36pm

On May 1, 2008, at 4:02 PM, Alan S. wrote:

‘task_list’, :state =>‘someday’
–Alan
A bit simpler as:

map.with_options :controller => ‘tasks’, :action =>‘task_list’ do |
tasks|
tasks.inbox ‘/inbox’, :state =>‘inbox’
tasks.actions ‘/actions’, :state =>‘actions’
tasks.waiting ‘/waiting’, :state =>‘waiting’
tasks.someday ‘/someday’, :state =>‘someday’
tasks.ticklers ‘/ticklers’, :state =>‘ticklers’
tasks.reference ‘/reference’, :state =>‘reference’
end

and you still get the named routes.

Or, junk having separate named routes and just do:

map.task_list ‘/:state’, :controller => ‘tasks’, :action =>‘task_list’,
:requirements => { :state => /\A(inbox|actions|waiting|someday|
ticklers|reference)\z/ }

and use it in a view like:
task_list_path(:state => ‘inbox’)

and in your controller:
params[:state]

-Rob

apswartz · May 2, 2008, 4:31pm

Can you please explain to me what /\A(…)\z/ does?

Thanks,

Alan

On Thu, May 1, 2008 at 3:30 PM, Rob B.

apswartz · May 2, 2008, 5:13pm

On May 2, 2008, at 10:30 AM, Alan S. wrote:

Can you please explain to me what /\A(…)\z/ does?

Thanks,

Alan

It’s a regular expression. \A matches the beginning of the string and
\z matches the end of the string. You might be more accustomed to
seeing ^ and $, but these match beginning and end of line and might
be confused by the presence of newlines in the string (granted that
such characters are unlikely in a URL component).

The use of the requirements is to keep “/nothing” from calling your
task_list with a params[:state]==‘nothing’.

-Rob

‘task_list’, :state =>‘actions’
In tasks_controller.rb, there is a task_list method
tasks.actions ‘/actions’, :state =>‘actions’
map.task_list ‘/:state’, :controller => ‘tasks’, :action
-Rob

Thanks,

wrote:

Rob B. http://agileconsultingllc.com
[email protected]
+1 513-295-4739
Skype: rob.biedenharn

apswartz · May 2, 2008, 5:17pm

Thanks!!

I tried this, but I got the following error:
/Library/Ruby/Gems/1.8/gems/actionpack-2.0.2/lib/action_controller/routing.rb:954:in
`assign_route_options’: Regexp anchor characters are not allowed in
routing requirements:
/\A(inbox|actions|waiting|someday|ticklers|reference)\z/
(ArgumentError)

When I remove the \A and \z, then it works just fine. If I pass it
/nothing, then I get a routing error. (as I would expect.)

–Alan

On Fri, May 2, 2008 at 10:12 AM, Rob B.

apswartz · May 2, 2008, 6:17pm

Hm, OK the code wins. I suppose it is possible for the anchors to be
implicit in the route matching code (I’ve never looked into it). If a
route like /spinboxes doesn’t match the state ‘inbox’ (sp·inbox·es),
then you’ll be fine – that’s all that I was trying to help you avoid.
-Rob

apswartz · May 2, 2008, 8:50pm

Thanks for your help!! I checked and spinboxes doesn’t match inbox…

-Alan

On Fri, May 2, 2008 at 11:16 AM, Rob B.