http://qs321.pair.com?node_id=61107


in reply to Juniper Router Audit

Like ichimunki, I won't really say much beyond style, and in particular, deep nested conditionals.

As one example of reducing the complexity of your nesting (but different than ichimunki's suggestion of using logical operators), here is an alternate version of the foreach loop found within the getintnames() routine -- your loop was 26 lines long and 4 levels deep (with a couple of unnecessary indentations added as well).

foreach my $fpc (@lines) { if ($fpc =~ /Slot (\d+?).*$/) { $slotcounter = 1; $slot = $1; next; } next unless $slotcounter eq 1; next unless ($cardtype, $pic) = $fpc =~ /PIC (\d+?)\s+(.*?)\,.*$/; next unless $cardtype =~ /$type/; ($ports = $cardtype) =~ s/^(\d)x.*/$1/; push @cardz, "so-$slot/$pic/$_" for 0 .. $ports - 1; }

Note, I removed your $knownthing variable altogether as you didn't really do anything with it. Also note, I can't really test the above for errors beyond syntax, so for that reason just consider it model rather than a plug-in alternative.

And a final note: my point was not to reduce the size of the loop, that was only a side-effect of reducing the nesting. However, some people frown at multiple points of escaping a loop iteration (though in this case the nested ifs amount to the same thing), so tmtowtdi and ymmv.

Replies are listed 'Best First'.
Re: Re: Juniper Router Audit
by cleen (Pilgrim) on Feb 28, 2001 at 07:25 UTC
    ah yes, that took down some of the complexity of the code quite a bit, and I will post the revision tommorow once I am in work again. Another thing I noticed is the fact that each time it finds a MATCH INTERFACE type, it goes and gets a relist of all of that type of card, which extends the overhead of the program with time and processing. I think the $knownthing was supposed to be part of that future logic, but it didnt get into a final revision =\ Thanks for your input it was greatly needed!