Assuming I was asking job candidates to write a short Perl program in order to rate their capacity, I would simply not hire somebody producing such code, even if the program result was correct, because this is just far too sloppy (I am sorry, don't take this a personal critique, but really as an advice and opportunity to improve). Now, the reality is that we are not doing Perl tests for new candidates, because we don't care that much if they know Perl; we use many technologies, Perl is just one of them, and they will have to learn it eventually. But the point is that I would not hire anyone writing such poorly formatted code in any programming language. In other words, to me, writing clean code is in my view a much more important asset than any specific language skills.
Using correct and consistent indentation (irrespective of various coding styles among which you might pick one or the other) is one of the keys to understand what you write, to make it understandable to the coworkers that will have to maintain your code and also to explaining your intentions to the reader (this is essential: if I know, through formatting, what you mean, then I have a fair chance to find a possible bug; if I do not even know what you intended to do, then there is a significant chance that I'll be lost). If you have to, use a code prettifier such as Perl::Tidy or perltidy, but I would encourage you to write tidy code in the first place, because it greatly helps you to better think your code.
Just a couple examples to explain. You have:
Everything that is within the opening brace at the end of the first line should be indented. In addition, although this is less important, if you want to insert vertical spaces, put them before the foreach line, not after. So this could be:my $i = 0; foreach my $match(@exonic_matches) { $value = $exon_ID[$i]; $col = $exon_left{$value}; #...
or possibly:my $i = 0; foreach my $match(@exonic_matches) { $value = $exon_ID[$i]; $col = $exon_left{$value}; #... }
depending on your preferred style.my $i = 0; foreach my $match(@exonic_matches) { $value = $exon_ID[$i]; $col = $exon_left{$value}; #... }
Similarly, your code:
would be much better rewritten this way:if ($strands{$value} =~ m/\+/) { $diff_three_prime = $LBP[$i] - $three_prime_ss[$exons2{$value} - 1 +]; $diff_five_prime = $LBP[$i] - $five_prime_ss[$exons2{$value} - 1]; + # ...
Just to state it once more, this is not a matter of taste, this is essential to good programming. I do not really care which formatting style you choose (K&R, GNU, Microsoft, ...), but the formatting has to be consistent and to show the structure of your program.if ($strands{$value} =~ m/\+/) { $diff_three_prime = $LBP[$i] - $three_prime_ss[$exons2{$value} + - 1]; $diff_five_prime = $LBP[$i] - $five_prime_ss[$exons2{$value} - + 1]; # ...
And, again, I am really not writing this to chastise you in any way, but to try to help you using better programming practices. Really do it, I am sure that you will see the benefits fairly quickly.
In reply to Re: looping foreach error
by Laurent_R
in thread looping foreach error
by Anonymous Monk
| For: | Use: | ||
| & | & | ||
| < | < | ||
| > | > | ||
| [ | [ | ||
| ] | ] |