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
| For: | Use: | ||
| & | & | ||
| < | < | ||
| > | > | ||
| [ | [ | ||
| ] | ] |