My first module: Blaster.pm

[Peter Scott]

At 10:59 AM 10/21/02 -0700, nkuipers wrote:
>Ok ok, I know this is silly of me, but I just wanted to show you all.  Not
>only does it compile, it works As Intended! :)  However, I would like to hear
>any suggestions anyone has for improving it...after all, it's just my first.

Well, you can save yourself a bit of typing; since bless returns the 
reference after blessing the referent, where you say

         bless ($self, $type) and return $self

you can just say

         return  bless ($self, $type);

For that matter, you could replace the whole

         my $self = {
                 TYPE => 'n,x,p etc.',
                 DATABASE => 'some db',
                 INPUT => 'some file',
                 OUTPUT => 'some file',
                 MODE => 'int(0-7)',
                 EVALUE => 'some value',
         @_
         };
         bless ($self, $type) and return $self

with

         bless {
                 TYPE => 'n,x,p etc.',
                 DATABASE => 'some db',
                 INPUT => 'some file',
                 OUTPUT => 'some file',
                 MODE => 'int(0-7)',
                 EVALUE => 'some value',
         @_
         }, $type

But that's just pruning.

         if ( $argument eq 'n'  or
              $argument eq 'p'  or
              $argument eq 'x'  or
              $argument eq 'tn' or
              $argument eq 'tx' ) { $caller->{TYPE} = $argument }
         else { die "Invalid -blast value: $!" }

spells hash test to me:

         my %valid = map { $_ => 1 } qw(n p x tn tx);
         $valid{$argument} ? $caller->{TYPE} = $argument
                         : die "Invalid -blast value: $argument";

And you don't really want $! there... that's only for system call failures.

sub blast {
         my $caller = shift;
         ref($caller) or die "Invoked instance method on a class: $!";
         print "Blasting against $caller->{DATABASE} database...\n";
         my $command = "blastall -p blast". $caller->{TYPE} .
                       " -d " . $caller->{DATABASE} .
                       " -i " . $caller->{INPUT} .
                       " -o " . $caller->{OUTPUT} .
                       " -m " . $caller->{MODE} .
                       " -e " . $caller->{EVALUE};
         system $command;
}

I think I'd want to know whether this was successful.  You've called 
system(), which means that the STDOUT and STDERR will go to the 
caller.  Maybe that's right; I don't know what blast is.  Usually one 
wants to hide the underlying details from the user since that's the 
encapsulation part of O-O.

So unless this is an interactive program or it outputs messages that 
you are going to print anyway, I'd captur the stdout and stderr with `` 
and do some analysis on them to see whether they represented success or 
failure.  Or maybe it's enough just to look at $? for the return code.

--
Peter Scott
Pacific Systems Design Technologies
http://www.perldebugged.com/