def self.find_left_sibling( parent_id, cat_name )
parent = Category.find( :first,
:conditions => [ "id = ?", parent_id] )
children = parent.children
return nil unless !children.nil?
left = Category.new
left.name = ''
left_id = nil
count = 0
children.each do |child|
if cat_name.between?( left.name, child.name )
left_id = left.id
break
end
left = child
count = count + 1
break unless cat_name > child.name
end
if left_id
return left_id
elsif count == children.length
return left.id
end
end
Refactorings
No refactoring yet !
Emmett
October 4, 2007, October 04, 2007 10:22, permalink
I'd recommend changing the function so that it returns the child itself, rather than an ID...and refactoring so that this is a function of instances of category. You probably don't need the class function after that, but I included it anyway.
I haven't tested this code, but it should work. It's possible I have the order wrong, and this gives you the right sibling instead. In that case, change "<" to ">" and remove "desc".
def self.find_left_sibling(parent_id, cat_name) parent = Category.find(parent_id) left_child = parent.find_child_left_of(cat_name) left_child ? left_child.id : nil end def find_child_left_of(cat_name) children.find :first, :conditions => ["name < ?", cat_name], :order => "name desc" end # if you have a parent_id field on categories, you could also have: # @left_sibling = @category.find_left_sibling def find_left_sibling parent.children.find :first, :conditions => ["name < ?", name], :order => "name desc" end
michiel
October 4, 2007, October 04, 2007 15:47, permalink
The previous refactoring is great. Just let the db do the work. Staying closer to the original, it could be a lot leaner. Don't bother with breaks, just return when you've found your match. If the first child's name > than the cat_name, there's no match. If you haven't found anything after the loop, it's the last child, if there are any children.
You can you inject to keep track of 'left' - you seed it with "inject(nil)" - the last expression becomes "left" in the next iteration, and you store the final iteration in the "left" local variable.
has_many relations don't return nil - no need to check. And "unless !" is better known as "if".
Cheers,
Michiel.
def self.find_left_sibling( parent_id, cat_name )
parent = Category.find( :first,
:conditions => [ "id = ?", parent_id] )
left = parent.children.inject(nil) do |left,child|
return left && left.id if cat_name < child.name
child
end
left && left.id
end
dmmaseoseoseo
December 12, 2011, December 12, 2011 10:48, permalink
I know it is difficult to define all about that theory that explains so poorly.
Set in array is in alpha order. Looking through set for position that cat_name would live. Want to exit the loop when I find the position.
This code does currently work correctly, but seems verbose and java-ish.