Re: Up for Critique

The first thing I would suggest is running 'monitor' while the program is running. This should give you an idea is the time is being spent in your Perl script or in the database. This would help you focus your optimization effort. I worked with bioinformatic data professionally myself, and the way I would typically do these type of problems is to pull in all the data I could into memory first (hashes work wonders), process your results in memory, and then do your table inserts and updates (updates first). You want to do multiple updates and inserts in a single transaction block, but be sure to commit every so often so your backtrack space doesn't fill up.

Here are a few coding comments I have:

Use $INPUT_RECORD_SEPARATOR instead of $/

We can eliminate a lot of lexical scoping on the parse routines since the m// function returns as a list the substring matched by the capturing parentheses. If there is no match, the list item will be undef.

    # Get type. Right now, all are BAC's.
    if ($line =~ m/\b((B|Y)AC)\b/)
    {
      $type = $1;
    }
    
    # Get reference number.
    if ($line =~ m/BAC:\s*(.*)\s*$/)
    {
      $ac_id = $1;
    }

    # Get whether forward or reverse
    if ($line =~ m/ORIENTATION:\s*(\w)_(\d)\s*$/)
    {
      $orient = $1;
      $read_frame = $2;
    }
[download]

becomes

    # Get type. Right now, all are BAC's.
    ($type) = $line =~ m/\b([BY]AC)\b/; # regex change
    
    # Get reference number.
    ($ac_id) = $line =~ m/BAC:\s*(.*)\s*$/;

    # Get whether forward or reverse
    ($orient, $read_frame) =
      $line =~ m/ORIENTATION:\s*(\w)_(\d)/; # regex change
[download]

Watch all the *'s (especially .*'s) in regex's, they may not all be necessary.

I think everything else has been said already. I'd like to reiterate the utter stupidity of the CS department not allowing you to take their classes.

Comment on Re: Up for Critique Select or Download Code

Replies are listed 'Best First'.
Re: Re: Up for Critique by davorg (Chancellor) on Mar 23, 2002 at 16:55 UTC
1. Use $INPUT_RECORD_SEPARATOR instead of $/ I was wondering why you thought this was a good idea. IMO the aliases from English.pm can end up making your code harder to follow - purely because they are used so rarely and most people are far more used to using the punctuation variables. Also there used to be a performance problem with `use English`. It made Perl think that you'd used $`, `$&` and `$'`. This would slow down all regex matches in your code. I think this has been fixed in recent Perls, but I'm not sure exactly when (or to what extent). -- <http://www.dave.org.uk> "The first rule of Perl club is you do not talk about Perl club." -- Chip Salzenberg	[reply]
Re: Up for Critique by admiralh (Sexton) on Mar 24, 2002 at 00:26 UTC
Because I was not familiar with $/ and had to look it up. :-) Seriously, since this code was written to be looked at by other (non-Perl) types, I figured the long name would help. Now if it seriously causes performance problems, then by all means use $/, but comment what you are doing. And using local to scope the change is a good idea. BTW, that was my first post to Perl Monks.	[reply]
Re: Re: Up for Critique by biograd (Friar) on Mar 23, 2002 at 07:39 UTC
Thanks for the comments admiralh. I must have missed the part about $INPUT_RECORD_SEPARATOR in Programming Perl. It is, after all, described on one of the most evil of pages (3rd ed.). ;) Question: Is 'monitor' similar to the *nix shell command 'time' ? I just discovered that one. -Jenn	[reply]
Re: Re: Re: Up for Critique by Anonymous Monk on Mar 23, 2002 at 15:09 UTC
Yes monitor is similar to time. If you want to get really blood and guts try strace which traces system commands and calls. Often when there is a bottle neck it can aid detection (quickly).	[reply]