Overall, this code seems well laid out and easy to read: that's a good start. I'll just run through it, top to bottom, and comment as I see appropriate. (I don't know your level of Perl — ask if something needs further explanation.)

#!/usr/local/bin/perl

This is a module: you don't need a shebang line.

use Carp qw/croak cluck/;

See Carp. carp() and croak() give warnings/errors with short messages; cluck() and confess() use long messages. The short message forms are usually what you want in a module and, as they're exported by default, you can just write: 

use Carp;

[download]
use feature qw/say switch/;

Remove this line because:

  • I can only find one instance of say and adding a \n to a print is no huge effort.
  • The 'switch' feature is experimental. (More on how to work around the code using it below.)
  • It's contrary to your later statement in the POD: "Every reasonable effort has been made to maintain compatibility with previous versions of Perl."
$|++;

Don't make this global for the entire module. Find out where you need it and localise it: 

{
    local $| = 1;
    # code needing unbuffered output here
}

[download]

Note that I've shown an assignment (=) instead of an increment (++). The $|++ is a well known idiom; however, it works on the assumption that $| >= 0 is TRUE but never tests it. If someone ever decided to do the reverse to get buffered output (i.e. $|--) you could easily run into problems (due to an undocumented feature, that's not correct - see discussion below). Using local $| = 1;, you know exactly what you've got and where you've got it.

See also autoflush() in IO::Handle.

sub codon_to_aa { ... }

Here you're using the experimental 'switch' feature I mentioned earlier. Instead of the given/when/.../default, you can use if/elsif/.../else or one of the forms shown in perlsyn - Basic BLOCKs. Here's another alternative that's closer to how your original code looks: 

$ perl -Mstrict -Mwarnings -le '
    sub codon_to_aa {
        my ($codon) = @_;

        for ($codon) {
            return 
                  /GC./i    ? "A"
                : /TG[TC]/i ? "C"
                : /GA[TC]/i ? "D"
                : do { print "Sorry"; "" };
        }
    }

    for (qw{GCT TGT XXX}) {
        print "$_ : ", codon_to_aa($_);
    }
'
GCT : A
TGT : C
Sorry
XXX :

[download]

This subroutine also contains the only say() I found. print() is obviously fine but I'm wondering if carp() or croak() might be more appropriate here.

sub connect_db { ... }

I would tend to restrict that function to just what its name implies (i.e. making a connection). It would return the database handle ($dbh) which could be reused for multiple queries.

sub read_fasta_file { ... }

I don't know why you've chosen to use sysopen. If you're sure you need it, mention the portability issues in the "INCOMPATIBILITIES" section of your POD. I think open would be fine here — use the 3-argument form.

sub usage { ... }

I'd say this is surplus to requirements. What you do need, however, is a "SYNOPSIS" section at the start of your POD with much the same information. You show GCCBioParser which does not match the package name. You show a new() constructor which does not exist and none of your subroutines appear to be object methods. Until I got down to here, I'd assumed this was intended to be a functional module, not an object-oriented one. Also, do not recommend to users of this module that they use Indirect Object Syntax — see perlobj - Invoking Class Methods.

sub AUTOLOAD { ... }

This seems unnecessary: perl will report if nonexistent functions are called.

sub DESTROY { ... }

Unless you're planning to add other functionality, this is also unnecessary: perl will free memory, you don't need to do it yourself.

-- Ken

In reply to Re: Bioinformatics project feedback request. by kcott
in thread Bioinformatics project feedback request. by Anonymous Monk

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post, it's "PerlMonks-approved HTML":


  • Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
  • Titles consisting of a single word are discouraged, and in most cases are disallowed outright.
  • Read Where should I post X? if you're not absolutely sure you're posting in the right place.
  • Please read these before you post! —
  • Posts may use any of the Perl Monks Approved HTML tags:
    a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, details, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, summary, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
  • You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.