Recursive method not working

[Zouplaz]

Hi, I'm trying to display a hierarchical tree but there's something
wrong with the method below.
The result var is 'lost' between the calls - I mean, each "#{result}"
contains the right value at the end of the method (I checked that), but
that value is lost when returning to the upper level.

I don't see why...

Thanks for your help !

def affiche_arbre(rubriques)
for rubrique in rubriques
result = '<br/>'
0.step(rubrique.level) { result += '&nbsp;'}
result += rubrique.libelle + ' ' + link_to('+', :action => 'new',
arent_id => rubrique) + ' '
unless rubrique.parent_id == nil
result += link_to('E', :action => 'edit', :id => rubrique) + '
' + link_to('D', :action => 'delete', :id => rubrique )
end
if rubrique.children.size > 0
result += affiche_arbre(rubrique.children)
end
end
"#{result}"
end

 

[Robert Klemme]

Hi, I'm trying to display a hierarchical tree but there's something
wrong with the method below.
The result var is 'lost' between the calls - I mean, each "#{result}"
contains the right value at the end of the method (I checked that), but
that value is lost when returning to the upper level.

I don't see why...

Thanks for your help !

def affiche_arbre(rubriques)

I'd try to insert

result=""

here.

for rubrique in rubriques
result = '<br/>'
0.step(rubrique.level) { result += '&nbsp;'}
result += rubrique.libelle + ' ' + link_to('+', :action => 'new',
arent_id => rubrique) + ' '
unless rubrique.parent_id == nil
result += link_to('E', :action => 'edit', :id => rubrique) + ' '
+ link_to('D', :action => 'delete', :id => rubrique )
end
if rubrique.children.size > 0
result += affiche_arbre(rubrique.children)
end
end
"#{result}"

Why do you do that? You could simply return result.

end

A general note: using << instead of += is much more efficient. You can
get even better by passing the result like this:

def meth(node, result="")
...
result << "START"
...
# recursion
meth(another_node, result)
...
result << "END"
...
result
end


Kind regards

robert

 

[Jano Svitok]

I'd try to insert

result=""

here.


Why do you do that? You could simply return result.


A general note: using << instead of += is much more efficient. You can
get even better by passing the result like this:

def meth(node, result="")
...
result << "START"
...
# recursion
meth(another_node, result)
...
result << "END"
...
result
end


Kind regards

robert

More remarks:

you should changeto
result << '<br/>'
as well

now some enhacements:
rubriques.each do |rubrique|
-- it's a question of style/taste otherwise equal
unless rubrique.parent_id.nil?
unless rubrique.children.empty?
-- this one may be faster sometimes (if size computation is slow)
and it more explicitly shows what are you asking
(rubrique.level + 1).times { result << '&nbsp;'}
or
result << ('&nbsp;' * (rubrique.level +1))
-- the first one is more descriptive, IMO

Jano

 

[Brian Candler]

Hi, I'm trying to display a hierarchical tree but there's something
wrong with the method below.
The result var is 'lost' between the calls - I mean, each "#{result}"
contains the right value at the end of the method (I checked that), but
that value is lost when returning to the upper level.

I don't see why...

Thanks for your help !

1> def affiche_arbre(rubriques)
2> for rubrique in rubriques
3> result = '<br/>'
4> 0.step(rubrique.level) { result += '&nbsp;'}
5> result += rubrique.libelle + ' ' + link_to('+', :action => 'new', arent_id => rubrique) + ' '
6> unless rubrique.parent_id == nil
7> result += link_to('E', :action => 'edit', :id => rubrique) + ' ' + link_to('D', :action => 'delete', :id => rubrique )
8> end
9> if rubrique.children.size > 0
10> result += affiche_arbre(rubrique.children)
11> end
12> end
13> "#{result}"
14> end

Well, the only recursive call I can see is line 10. You append the return
values from these calls onto 'result'. But then in line 12 you go back
around the loop, and in line 3 you reinitialise result, thereby throwing
away everything you've calculated so far.

 

[Zouplaz]

le 18/04/2007 21:18, Brian Candler nous a dit:

Well, the only recursive call I can see is line 10. You append the return
values from these calls onto 'result'. But then in line 12 you go back
around the loop, and in line 3 you reinitialise result, thereby throwing
away everything you've calculated so far.

You got it ! Thank you !

 

[jsnX]

def affiche_arbre(rubriques)
for rubrique in rubriques
result = '<br/>'
0.step(rubrique.level) { result += '&nbsp;'}
result += rubrique.libelle + ' ' + link_to('+', :action => 'new',
arent_id => rubrique) + ' '
unless rubrique.parent_id == nil
result += link_to('E', :action => 'edit', :id => rubrique) + '
' + link_to('D', :action => 'delete', :id => rubrique )
end
if rubrique.children.size > 0
result += affiche_arbre(rubrique.children)
end
end
"#{result}"
end

This code could also be:
============ code ============
links(rubrique)
[ link_to('+', :action => 'new', arent_id => rubrique) ] +
( rubrique.parent_id ?
[ link_to('E', :action => 'edit', :id => rubrique),
link_to('D', :action => 'delete', :id => rubrique) ] : [ ] )
end

def affiche_arbre(rubriques)
rubriques.collect do |rubrique|
'<br/>' + ('&nbsp;' * rubrique.level) +
[ rubrique.libelle ].concat(links(rubrique)).join(' ') +
( rubrique.children.size.zero? ? '' :
affiche_arbre(rubrique.children) )
end.join()
end
=========== /code ===============
Please excuse the Anglicized new function -- I know no French. Notice
that this code allows you to test correctness of link generation
separately from correctness of the generated HTML. By restricting
myself to expressions -- there are no variable assignments in this
code, let alone mutations -- I protect myself from the kind of bug
that marred the original.