misbehaving program

Anonymous Monk has asked for the wisdom of the Perl Monks concerning the following question:

Hi, I have a script that it sort of working, but not properly snd I can't really see why. First of all, it seems that although the OUTFILE's are being created, they are only created if the input file is below a certain size. So, for example, it will run on a input file with 500 entires but not 4000, ehich is what I want.

Secondly, its not deleting the .out and .dssp files at the end .. the naming strategy is correct as far as I am aware.

I would very much appreciate someone looking at my code to see where the problems might be. Thanks in advance.

#!/usr/bin/perl

$pdbprep = "/home/users/alison/pdb_folder/pdb";
$pdbext = ".ent";
$dssp = ".dssp";
$pdblist = $ARGV[0];
$combodir = $ARGV[1];
$out = ".out";

open(PDBLIST, "$pdblist") || die "ERROR: Unable to open $pdblist for r
+eading: $!\n";

while(<PDBLIST>)
{
    
    my @fields = split;
    
    my $pdbcode = $fields[0];

    $pdbcode =~ tr/[A-Z]/[a-z]/;
    
    my $dsspfile = "$pdbcode$dssp";
    
    my $pdbstructure = "$pdbprep$pdbcode$pdbext";

    
    $residuecombofile = "$combodir$residue1$residue2$residue3$out";

    $outfile = "$pdbcode$out";

    if(open(PDB,"$pdbstructure"))
    {
    system("dsspcmbi $pdbstructure $dsspfile"); 
    CreateUniqueResidueFile($pdbstructure, $outfile);
    
    }
    else
    {
    $flag = 1;

    }

    if($flag == 0)
    {
    open(UNIQUE,"$outfile") || die "ERROR: Unable to open $outfile for
+ reading: $!\n";

    @file = <UNIQUE>;

    $items = @file;
        
    for($j = 0; $j <$items-2; $j++)
    {
        $residuenumber1 = substr($file[$j], 1, 4);
        
        $residuename1 = substr($file[$j], 7, 3);
        
        $residuenumber2 = substr($file[$j+1], 1, 4);
        
        $residuename2 = substr($file[$j+1], 7, 3);
        
        $residuenumber3 = substr($file[$j+2], 1, 4);
        
        $residuename3 = substr($file[$j+2], 7, 3); 
        
        $residuecombofile = "$combodir$residuename1$residuename2$resid
+uename3$out";
        
        ($phiangle, $psiangle) =  ExtractFromDssp($residuenumber1, $re
+siduenumber2, $residuenumber3, $dsspfile);

            
        if($phiangle != 0)
        {
        if($phiangle != 360)
        {
            if($psiangle !=360)
            {
                
            open(OUTFILE, ">>$residuecombofile") || die "ERROR: Unable
+ to open $residuecomdofile for writing: $!\n";
            
            print OUTFILE "$phiangle\t$psiangle $pdbcode\n";
            }
        }
        }
    }
    
    
    system("rm $dsspfile");
    system("rm $outfile");        
            
    }
    else
    {
    print "No structure file. Moving to next structure in list\n";
    }             
    
    
}
[download]

Janitored by Arunbear - added readmore tags, as per Monastery guidelines

Comment on misbehaving program Download Code

Replies are listed 'Best First'.
Re: misbehaving program by reasonablekeith (Deacon) on Apr 20, 2005 at 10:39 UTC
put `use strict; use warnings;` [download] at the top of your script, then go through and declare all your variables as your script will no longer compile as is. Then sort out your indenting and see if you can spot your errors. Also try changing system for backticks so you can log any errors, or at least check the return code from system ($!). `perldoc -f system` [download]	[reply] [d/l] [select]
Re: misbehaving program by reneeb (Chaplain) on Apr 20, 2005 at 10:44 UTC
First of all you should use `use strict;` and `use warnings`, then look at BioPerl. There are modules to read pdb-files! good slides: http://www-users.cs.umn.edu/~karypis/Classes/CS5481/Misc/Intro_bioperl.ppt to delete files you should use `unlink` (perldoc -f unlink) and you should close filehandles explicit.	[reply] [d/l] [select]
Re^2: misbehaving program by Anonymous Monk on Apr 20, 2005 at 11:21 UTC
Hi there Thanks for the help so far. I am aware of bioperl, but wouldn't touch it with a barge pole quite honestly. I'll see how I get on as regards to the other suggestions though.	[reply]
Re: misbehaving program by bofh_of_oz (Hermit) on Apr 20, 2005 at 13:23 UTC
for deleting .out and .dssp files, use unlink: `unlink $dsspfile; unlink $outfile;` [download] As for creating OUTPUT file, it seems like you try to append to the file instead of writing to it. If you are creating the file and writing to it, you can try this: `open(OUTFILE, "> $residuecombofile") \|\| die "ERROR: Unable to open $re +siduecomdofile for writing: $!\n" print OUTFILE "$phiangle\t$psiangle $pdbcode\n"; ... close OUTFILE;` [download] Hope this helps...	[reply] [d/l] [select]
Re: misbehaving program by tlm (Prior) on Apr 20, 2005 at 13:11 UTC
As others have pointed out you are not running under `use strict` and `use warnings`, and you are not checking the return status of your `system` calls. For the latter you should at least have something like `system("dsspcmbi $pdbstructure $dsspfile") and die "System called fail +ed.";` [download] to alert you that something went awry, and where. (Note that `system` is the unusual Perl function that returns a true value upon failure, hence the `and` clause to check for failure.) Next, the line `$residuecombofile = "$combodir$residue1$residue2$residue3$out";` [download] is executed redundantly at a couple of places throughout the program (as far as I can tell, the RHS always evaluates to the same value); moreover, `$residue1, $residue2,` and `$residue3` are undefined. (BTW, on the naming of variables, you may want to read this node.) In your post you refer to "`OUTFILE`'s" but it is clear that this program will generate at most one `OUTFILE`, whose name is the constant value of `$residuecombofile`. You open a file handle `PDB` with this line `if(open(PDB,"$pdbstructure"))` [download] but you don't do anything with the handle. If all you want to do is check for the existence of this file, use `if ( -e $pdbstructure )` [download] or better yet `if ( -r $pdbstructure )` [download] (See file test operators). Furthermore, if the `open` cited above fails once, then it sets `$flag`, which would stay set, since it is never reset back to 0 anywhere (in fact, this variable is never properly initialized outside of the loop); this means that, after one failure of the `open`, the then clause of the `if` statement beginning with `if($flag == 0)` [download] would never again be executed, even if subsequent calls calls to `open(PDB,"$pdbstructure")` structure succeed. (BTW, the idiomatic way to make the test above is `unless ( $flag )`.) Do you ever see the message `No structure file. Moving to next structure in list` [download] ? the lowliest monk	[reply] [d/l] [select]
Re^2: misbehaving program by BazB (Priest) on Apr 22, 2005 at 23:48 UTC
My brain doesn't cope well with `system("command") and die "Error message: $?";` [download] Although the above is perfectly valid - as system returns zero for success, unlike every other perl function I can think of - I'd much rather use: `system("command") == 0 or die "Error message: $?";` [download] ...which makes it much more obvious (at least to me) what's going on. Although I note that the examples in the perldoc use the latter approach, this is mostly a personal style point/grumble: feel free to ignore it. If the information in this post is inaccurate, or just plain wrong, don't just downvote - please post explaining what's wrong. That way everyone learns.	[reply] [d/l] [select]