[tpm] OID sorting

[Henry Baragar]

On May 1, 2009 03:15:19 pm Uri Guttman wrote:
> >>>>> "HB" == Henry Baragar <Henry.Baragar at instantiated.ca> writes:
>
>   HB> #! /usr/bin/perl -w
>
> use warnings is better these days.
>   HB> use strict;
>
>   HB> sub oid_tree_add {
>   HB>     my ($tree, $oid, $value) = @_;
>   HB>     my @oid = split /\b/, $oid;
>
> why are you splitting on \b? you could just split on /\./ and get the
> oid numbers. then you can simplify (and speed up) the code below.
>
You are correct (the code evolved and I did not finish refactoring).

See attached with this refactoring.

>   HB>     _oid_node_add($tree,$value, at oid);
>   HB> }
>
>   HB> sub _oid_node_add {
>   HB>     my ($tree, $value, $key, @suboid) = @_;
>   HB>     die "Expecting integer, got '$key'" unless $key =~/^\d+/;
>   HB>     my $node = $tree->{$key};
>   HB>     if (my $dot = shift @suboid) {
>   HB> 	die "Expecting '.', got '$dot'" unless $dot eq ".";
>   HB> 	$node->{subtree} = _oid_node_add($node->{subtree}, $value, @suboid);
>
> i won't get into detail here but if you split on . as i said above, you
> can just delete the check for dot. i can't see any reason to keep that
> logic.
>
>   HB>     }
>   HB>     else {
>   HB> 	$node->{value} = $value;
>   HB>     }
>   HB>     $tree->{$key} = $node;
>
> i don't see $node being modified anywhere. you modify its keys/values
> but $node is still the same hashref. so that line probably could go.
It simply reads better (IMHO).

>   HB>     $tree;
>
> use explicit return statements vs implicit last value returns. this is a
> bug waiting to happen if you ever add/modify code in that section. you
> may forget what was being returned and return some other expression.
>
>   HB> }
>
>   HB> sub oid_tree_sorted {
>   HB>     my ($tree) = @_;
>   HB>     my @sorted;
>   HB>     for my $key (sort {$a <=> $b} keys %$tree) {
>   HB> 	my $node = $tree->{$key};
>   HB> 	push @sorted, $node->{value} if $node->{value};
>   HB> 	push @sorted, oid_tree_sorted($node->{subtree})
>   HB> 	    if $node->{subtree};
>
> if a node has a value it won't have a subtree. 
No, a node is allowed to have both (this is not a pure tree).

> so you don't need the
> second test. in general you could have eliminated the need for the
> value/subtree keys and just used the value or hash ref. then you can
> tell if it is a value or tree node with ref(). it is a simpler and
> cleaner data tree design. again, i could be wrong in your details but
> this is a common design idea in perl.
>
True, if you have a pure tree (which we don't)

> i leave the actual rewrite as an exercise as i am not up for it at the
> moment. :)
>
> thanx,
>
> uri

Regards,
Henry

-- 

Henry Baragar
Instantiated Software
416-907-8454 x42
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.pm.org/pipermail/toronto-pm/attachments/20090501/1ffb3cf4/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: oid_sort
Type: application/x-perl
Size: 1212 bytes
Desc: not available
URL: <http://mail.pm.org/pipermail/toronto-pm/attachments/20090501/1ffb3cf4/attachment.bin>