>>>>> "SB" == Steve Bertrand <st...@ibctech.ca> writes:

  SB> Uri Guttman wrote:
  >>>>>>> "t" == trapd00r  <trapd...@trapd00r.se> writes:
  >> 
  t> I would like to have someone looking over my script, which is a
  t> basic frontend for playing radio with mplayer. In particular, I'm
  t> wondering how I could get rid of all those elsif's when parsing the
  t> arguments; as you can see, there's lots of them, and I suspect that
  t> there's a better way of doing things.
  >> 
  t> Any criticism on anything is highly appreciated, since I want to learn.
  t> Cheers.
  >> 
  >> i see an error on line 42.

  SB> I knew you were good, but now I know you are great!

hmmm....


  SB> #...use() calls here

  SB> my %arg_table = (

  SB>         '--plans'           => \&plans,
  SB>         '--uledger'         => \&uledger,
  SB>         '--gledger'         => \&gledger,
  SB>         '--balance'         => \&balance,
  SB>         '--clients'         => \&clients,
  SB>         '--plan_passwords'  => \&plan_passwords,
  SB>     );

  SB> my $convert = \%arg_table;

why do you need that ref?

  SB> my @allowed_args = keys %arg_table;

  SB> if ( ! defined $ARGV[0] || $#ARGV > 1 ) {

  SB>     print "\n" .
  SB>           "You must supply one of:\n\n" .
  SB>           "${\( join \"\n\", @allowed_args ) }" .
  SB>           "\n\n";
  SB>     exit;
  SB> }

  SB> my $item_to_translate = $ARGV[0];

generally, grab @ARGV stuff near the top. just like @_ processing, get
it done as early as possible for both clarity and safety (as in no code
can mess with it before you get to it).

  SB> # call the requested conversion function

  SB> $convert->{$item_to_translate}();

$convert is not needed:

        $arg_table{$item_to_translate}->() ;

i don't know exactly what your code is doing since you committed the
same sin as the OP, not posting (all of) the code. but my guess is that
you are processing the args as you find them. this is a weak design in
general as i learned way back in my c days. args (from any source,
command line or calls) should be dealt with in
3 phases:

        collection gets the args into the variables you want.

        checking them for validity (and exeiting if needed) and setting
        defaults

        executing the desired commands based on the args.

the reason is that you may execute immediately on one arg but later find
out another is invalid. the example i recall was a coder working for me
did this. and the executed code did a lot of work immediately and then
he still had to bomb out on another bad arg. he did the work before he
fully checked all the args. it wasn't fatal in that case but it wasted
cpu and could have been worse. so this is just a good idea when
processing args.

uri

-- 
Uri Guttman  ------  u...@stemsystems.com  --------  http://www.sysarch.com --
-----  Perl Code Review , Architecture, Development, Training, Support ------
---------  Gourmet Hot Cocoa Mix  ----  http://bestfriendscocoa.com ---------

-- 
To unsubscribe, e-mail: beginners-unsubscr...@perl.org
For additional commands, e-mail: beginners-h...@perl.org
http://learn.perl.org/


Reply via email to