Forum: Ruby Recursive method not working

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
Zouplaz (Guest)
on 2007-04-18 20:05
(Received via mailing list)
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',
: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
Robert K. (Guest)
on 2007-04-18 20:16
(Received via mailing list)
On 18.04.2007 18:01, Zouplaz wrote:
> def affiche_arbre(rubriques)
I'd try to insert

result=""

here.

>         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 S. (Guest)
on 2007-04-18 20:46
(Received via mailing list)
On 4/18/07, Robert K. <removed_email_address@domain.invalid> wrote:
> >
> >       0.step(rubrique.level) { result += '&nbsp;'}
> >     "#{result}"
>    result << "START"
> 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
Brian C. (Guest)
on 2007-04-18 23:19
(Received via mailing list)
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 = '<br/>'
 4>       0.step(rubrique.level) { result += '&nbsp;'}
 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.
Zouplaz (Guest)
on 2007-04-19 00:26
(Received via mailing list)
le 18/04/2007 21:18, Brian C. 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 (Guest)
on 2007-04-19 11:15
(Received via mailing list)
On Apr 18, 9:01 am, Zouplaz <removed_email_address@domain.invalid> wrote:
>        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|
    '<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.
This topic is locked and can not be replied to.