From: Jano Svitok <jan.svitok@...>
Date: 2007-04-19T01:45:28+09:00
Subject: Re: Recursive method not working

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 = '<br/>'
> >       0.step(rubrique.level) { result += '&nbsp;'}
> >       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 = '<br/>'
to
result << '<br/>'
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 += '&nbsp;'}
(rubrique.level + 1).times { result << '&nbsp;'}
or
result << ('&nbsp;' * (rubrique.level +1))
-- the first one is more descriptive, IMO

Jano