in reply to improov mein k0den
Well, you're clearly a C programmer, since you use printf unnecessarily and you use the C-style for loop. :-)
I'd say you've done a fairly conventional procedural decomposition of the task. You have four or five subroutines, but they're very task specific (that is, they're not resuable) because they either have hard-coded values or they share a dependency on some external data. One good solution in cases like this is to go object oriented.
But that's not the only possible approach. My preference, at least for something this simple, would be to genericize the subroutines to eliminate their hard-coded data and their dependency on external variables, making all that stuff parameters instead. And really, there's no reason for so many subroutines. One will do.
#!perl use strict; use warnings; exec 'ssh', join '@', 'brannont', menu( [qw( secdevfdsapp01 dopey bonnie )], "\n\nssh hosts\n---------\n", "\n\nssh to: " ); # returns one of the items in the array passed as the first arg sub menu { my( $ar, $hdr, $prompt ) = @_; local @ARGV; { print $hdr if defined $hdr; print "[$_] $ar->[$_]\n" for 0 .. $#{$ar}; print $prompt if defined $prompt; my $c = <>; chomp $c; return $ar->[$c] if $c =~ /^\d+$/ && $c >= 0 && $c <= $#{$ar}; print "invalid choice\n"; redo; } }
I eliminated your printf and C-style for loop. I also fixed a bug: testing of the user input should first check that the string is a number before doing numeric tests on it. Also, by unsetting @ARGV, I ensure that input is from the console. You may or may not want this.
|
|---|
| Replies are listed 'Best First'. | |
|---|---|
|
Re^2: improov mein k0den
by jettero (Monsignor) on Mar 29, 2007 at 18:59 UTC |