http://www.perlmonks.org?node_id=282753


in reply to Batch Router

The first comment I would make is "if it works, don't fix it". The next comment would be that if you find you do need to fix it at some point, or need to write something else of a similar nature, doing it with fewer lines will generally be better. I'm not pushing for a crypto/obfu/golf style -- just use a few more of the syntactic options that perl offers for the sake of clear brevity; e.g.:
# instead of this: if ($user_name eq "") { print STDOUT "User name cannot be blank.\n"; exit 1; } # do this die "User name cannot be blank.\n" unless ( $user_name ); # (numerous examples of that sort) #instead of this: my $out_FH=""; if ($output_file eq "") { $out_FH = \*STDOUT; } else { unless (open OUT_FH,">$output_file") { return 0; } $out_FH=\*OUT_FH; } return $out_FH; # do this: my $out_FH = ( $output_file eq "" ) ? \*STDOUT : ( open( OUT_FH, ">$output_file" )) ? \*OUT_FH : undef; return $out_FH; # and similarly, instead of this: if (!defined($conf_groups->{$ln_group})) { return 0; } return 1; # do this: return ( defined( $conf_groups->{$ln_group} ));
You could reduce your "Open_input_file" a lot by simplifying the error checking: don't bother with "-s", just read the file into @list (grepping off unwanted lines (blanks/comments) as needed) chomp @list, and return it, and have the caller check for an empty list.

You could also put some of the option strings into a hash (user_name, password, input_file, etc) -- since you prompt for these when they aren't stated on the command line, it will be easier to loop through the hash elements to do the "if(empty){prompt,read,check}" stuff, rather than coding a separate block to do this for each parameter; e.g.:

for my $opt ( qw/user_name password input_file/ ) { next if ( $opthash{$opt} ); print "You need to specify a $opt: " if ( $opt eq "password" ) { ReadMode( "noecho" ); else { ReadMode( "normal" ); } $opthash{$opt} = ReadLine(0); ReadMode( "restore" ); die "You really should have typed something for $opt\n" unless ( $opthash{$opt} ); } # this input mechanism only needs to be written once
Finally, when you have a long usage message to print, this sort of "here-document" layout is easiest:
my $Usage = <<ENDUSE; This is a great program and I'd like to fill up your console with pithy lines of information about all the stuff it can do. blah blah blah blah ENDUSE

Replies are listed 'Best First'.
Re: Re: Batch Router
by crackotter (Beadle) on Aug 18, 2003 at 18:21 UTC
    Another great idea, One of the things I need to work on is reducing my code bloat. This piece of work is accutly quite short for me (bloat wise). I will redo the script with the ideas you offered. Thanx