in reply to help with hash or arrays or hash references

That big if/elsif/elsif/else block is broken, here is why/how 

$ perl -e " use strict; use warnings; if( 0 ){ my $foo = 1; } else { $
+foo = 2; } "
Global symbol "$foo" requires explicit package name at -e line 1.
Execution of -e aborted due to compilation errors.

$ perl -e " use strict; use warnings; if( 1 ){ my $foo = 1; } else { $
+foo = 2; } "
Global symbol "$foo" requires explicit package name at -e line 1.
Execution of -e aborted due to compilation errors.

[download]

The  my $foo only exists inside that first if-block, all the other $foo refer to the undeclared global variable $foo

somebody told you to use strict, and you gave it a shot, but since you did not understand the benefits, you eventually turned it off , and now your program is broken

The shortest fix to satisfy strict is to move  my $foo outside the if/else/block, as in 

$ perl -e " use strict; use warnings; my $foo; if( 0 ){ $foo = 1; } el
+se { $foo = 2; } "

[download]

More on scope/strict in Lexical scoping like a fox, Read this if you want to cut your development time in half! and understand that  strict itself confers no benefits; The benefits come from avoidance of the bad practices forbidden by  strict :)

My second piece of advice, is turn that if/else block into a function, maybe like so 

    my( $Flag1, $Flag2 ) = ProtsFlags( $Subj_id, \%proph_prots, \%euk_
+prots, \%vir_prots );

    $HoFlg1{$Query_id}{$Subj_id}{$bits} = $Flag1;
    $HoFlg2{$Query_id}{$Subj_id}{$bits} = $Flag2;

[download]

Replies are listed 'Best First'.
Re^2: help with hash or arrays or hash references
by AWallBuilder (Beadle) on May 08, 2012 at 13:22 UTC

    Great Thanks. I had just realized the that I had to move the declaration to outside of the if else loop. I also like the idea of a subroutine. Subroutine is now working. Here working code for any following this thread

    new code

    ############### read in blast table and parse #######
my $in_blast_tab=$ARGV[0];
open(IN,$in_blast_tab) or die "cannot open $in_blast_tab\n";

my %HoFlg1; my %HoFlg2;
#my $Flag1; my $Flag2;

while (my $line=<IN>) {
        chomp $line;
        my ($Query_id,$strand,$Subj_id,$Perc_iden,$align_len,$num_mm,$
+gap,$q_start,$q_end,$s_start,$s_end,$e_value,$bit_score)=split("\t",$
+line); # extra field of strand
        next if ($bit_score <60);
        my ($Flag1, $Flag2 ) = &Flag( $Subj_id, \%proph_prots, \%euk_p
+rots, \%vir_prots );
        $HoFlg1{$Query_id}{$Subj_id}{$bit_score}=$Flag1;
        $HoFlg2{$Query_id}{$Subj_id}{$bit_score}=$Flag2;
        print join("\t",$Query_id,$Subj_id,$bit_score,$Flag1,$Flag2)."
+\n";
}

sub Flag {
        my $Subj_id=$_[0];
        my $proph_prots=$_[1];
        my $euk_prots=$_[2];
        my $vir_prots=$_[3];
        my $Flag1; my $Flag2;
        if (exists $proph_prots{$Subj_id}){
                $Flag1="Proph";
                $Flag2="Phage";
        }
        elsif (exists $euk_prots{$Subj_id}){
                $Flag1="Euk";
                $Flag2="Euk"
        }
        elsif(exists $vir_prots{$Subj_id}){
                $Flag1="Vir";
                $Flag2="Phage";
        }
        else {
                $Flag1="Bact";
                $Flag2="Bact";
        }
        return($Flag1,$Flag2);
}

    [download]
[reply]
[d/l]
      The hashes, %proph_prots, %euk_prots, %vir_prots are not initialized with any values.

      If the only parameters you are using from the file are:
      $Query_id, $Subj_id, $bit_score,
      you could make your initialization line as:
      my ($Query_id, $Subj_id, $bit_score) = (split /\t/, $line)[0, 2, -1];

      Update: Those are hash refs in the Flag() function but they are being used with regular hash notation/syntax. (You have not declared use strict; at the beginning of the script. Thats how I found the uninitialized hashes mentioned above.)

[reply]
[d/l]
[select]

      By chance, are ALL your resulting Flags "Bact"?

      You are passing in hash references, but not dereferencing them; thus it seems your IFs will always fail.

      Seems to me you should have this: 

         #if (exists $proph_prots{$Subj_id}){
    if (exists $$proph_prots{$Subj_id}){

      [download]

      And so on.

[reply]
[d/l]

      Here is another way to write that, less typing, fixed to dereference properly ( references quick reference ) , and filtered through perltidy -csc -otr -opr -ce -nibc -i=4 

      sub Flag {
    my ( $Subj_id, $proph_prots, $euk_prots, $vir_prots ) = @_;
    return "Proph", "Phage" if exists $$proph_prots{$Subj_id};
    return "Euk",   "Euk"   if exists $$euk_prots{$Subj_id};
    return "Vir",   "Phage" if exists $$vir_prots{$Subj_id};
    return "Bact",  "Bact";
} ## end sub Flag

      [download]
[reply]
[d/l]
        Thanks for tidying this up. I was not getting just Bact before, so I'm not sure why I didn't have to dereference? I have now changed it to $$ but there is no difference in the output. Sorry I did actually have use strict, but it is at the very beginning of the script which I didn't post.
[reply]

In Section Seekers of Perl Wisdom