CODEWITHSUNDEEP
ruby-on-railsrubyruby-on-rails-3
Tintin81
Tintin81

Reputation: 10207

How can this bit of Rails code be refactored?

Just out of curiosity:

How could this (rather ugly) Rails code be prettified / refactored:

def section_link(name, path)    
  link = link_to(name, path) 
  if name != controller.controller_name.titlecase
    link
  else
    link_to(name, path, :class => 'current')
  end
end

Upvotes: 1

Views: 148

Answers (5)

tokland
tokland

Reputation: 67860

I'd write:

def section_link(name, path)
  is_current = (name == controller.controller_name.titlecase)
  link_to(name, path, :class => ('current' if is_current))
end

Justification: 1) The variable is_current makes the code somewhat more declarative. 2) link_to assumes that nil means empty class (what we want here).

Upvotes: 5

sawa
sawa

Reputation: 168101

def section_link(name, path)    
  link_to(name, path,
  *({class: "current"} if name == controller.controller_name.titlecase))
end

Upvotes: 0

Zach Kemp
Zach Kemp

Reputation: 11904

You could do something like this:

def section_link(name, path)
  link_to(name, path, class: name == controller.controller_name.titlecase ? "current" : nil)
end

But that's getting a bit hard to read. I would split the class determination into another method:

def section_link(name, path)
  link_to(name, path, class: class_for(name) )
end

def class_for(name)
  name == controller.controller_name.titlecase ? "current" : nil
end

Upvotes: 1

Andrew Haines
Andrew Haines

Reputation: 6644

def section_link(name, path)
  options = {}
  options[:class] = 'current' if name == controller_name.titlecase
  link_to name, path, options
end

Upvotes: 6

Michael Koper
Michael Koper

Reputation: 9781

def section_link(name, path)     
  if name != controller_name.titlecase
    link_to(name, path)
  else
    link_to(name, path, :class => 'current')
  end
end

Or something like this

def section_link(name, path)
  link_to(name, path, :class => "#{"current" if name == controller_name.titlecase }")
end

Dont think it really needs refactoring tho, if it works...

Upvotes: 0

Related Questions