You'll have to update your node fetching routine but I fixed up the bugs in your _build routine. As for fetch routine itself - it is highly unperly in that instead of just returning @descendants you create some string that would have to be processed by the script user to get the children. I happen to think I had a very nice solution in Re: recursive complex structure or something like that? since it doesn't require the moduler writer to manually dereference the pointers. What you did was swap my weak references for the equivalent of symbolic references and in general I think having real references is a better idea.

Also note that your fetch routine can be forced into an infinite loop if there is a cycle in the parent/child relationships. On an overall quality level, you need to use better variable names than @fields. Your identifiers should be descriptive. Heck, its part of your documentation.

sub _build { my ($self, $file) = @_; croak "Unable to retrieve data" unless open(INPUT,$file); # You just stomped on the global $_. You have to either assign it # to a pad variable (like I did here) or local $_ first. Just a # plain while(<FH>) is dangerous. while (my $line = <INPUT>) { # You didn't pick named variables. Variables should have meani +ngful # names so you don't have to guess at what $field[2] is later +in # the code. my ($id, $parent_id, $title) = split ' ', $line; unless ( $id =~ /^\d+$/ and $parent_id =~ /^\d+$/ ) { carp "Unable to parse\n$line"; next; } # You were assigning the id to descendants. That should be sto +red # separately. Also, you used the hash keys of the object for t +he # node titles but also expect the 'decendents' key to remain u +nused. # You should keep that separate somewhere. $self->{'title2id_dict'}{$title} = $id; # Shall we assume the IDs all start low and go up? You'll cons +ume # much memory if an ID comes in like 1000000. I compensate for + that # and just use a hash. push @{$self->{'descendants'}{$parent_id}}, $id; } }

In reply to Re: Code Review (recursive data structure) by diotalevi
in thread Code Review (recursive data structure) by Limbic~Region

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post, it's "PerlMonks-approved HTML":



  • Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
  • Titles consisting of a single word are discouraged, and in most cases are disallowed outright.
  • Read Where should I post X? if you're not absolutely sure you're posting in the right place.
  • Please read these before you post! —
  • Posts may use any of the Perl Monks Approved HTML tags:
    a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, details, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, summary, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
  • You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.