Asp Forum - Recursive method not working

user@domain.invalid

4/18/2007 4:01:00 PM

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 = ' '
0.step(rubrique.level) { result += ' '}
result += rubrique.libelle + ' ' + link_to('+', :action => 'new',
:parent_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

5 Answers

Robert Klemme

4/18/2007 4:13:00 PM

On 18.04.2007 18:01, Zouplaz wrote:
> 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 = ' '
> 0.step(rubrique.level) { result += ' '}
> result += rubrique.libelle + ' ' + link_to('+', :action => 'new',
> :parent_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

4/18/2007 4:45:00 PM

On 4/18/07, Robert Klemme <shortcutter@googlemail.com> wrote:
> On 18.04.2007 18:01, Zouplaz wrote:
> > 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 = ' '
> > 0.step(rubrique.level) { result += ' '}
> > result += rubrique.libelle + ' ' + link_to('+', :action => 'new',
> > :parent_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
>
>

More remarks:

you should change
> > result = ' '
to
result << ' '
as well

now some enhacements:

> > for rubrique in rubriques
rubriques.each do |rubrique|
-- it's a question of style/taste otherwise equal

> > unless rubrique.parent_id == nil
unless rubrique.parent_id.nil?

> > if rubrique.children.size > 0
unless rubrique.children.empty?
-- this one may be faster sometimes (if size computation is slow)
and it more explicitly shows what are you asking

> > 0.step(rubrique.level) { result += ' '}
(rubrique.level + 1).times { result << ' '}
or
result << (' ' * (rubrique.level +1))
-- the first one is more descriptive, IMO

Jano

Brian Candler

4/18/2007 7:19:00 PM

On Thu, Apr 19, 2007 at 01:05:05AM +0900, Zouplaz wrote:
> 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 = ' '
4> 0.step(rubrique.level) { result += ' '}
5> result += rubrique.libelle + ' ' + link_to('+', :action => 'new', :parent_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.

user@domain.invalid

4/18/2007 8:23:00 PM

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

4/19/2007 7:12:00 AM

On Apr 18, 9:01 am, Zouplaz <u...@domain.invalid> wrote:
> def affiche_arbre(rubriques)
> for rubrique in rubriques
> result = ' '
> 0.step(rubrique.level) { result += ' '}
> result += rubrique.libelle + ' ' + link_to('+', :action => 'new',
> :parent_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', :parent_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|
' ' + (' ' * 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.

comp.lang.ruby

Recursive method not working

user@domain.invalid

Robert Klemme

Jano Svitok

Brian Candler

user@domain.invalid

jsnX

x Login to ForumsZone