CGI/Perl Guide | Learning Center | Forums | Advertise | Login
Site Search: in

  Main Index MAIN
Search Posts SEARCH
Who's Online WHO'S
Log in LOG

Home: Perl Programming Help: Beginner: Re: [lilmark] Moving part of Multi-Dimensional Array to an Array: Edit Log

Veteran / Moderator

Jan 1, 2013, 8:07 AM

Views: 4456
Re: [lilmark] Moving part of Multi-Dimensional Array to an Array

Compressing your code down to fewer lines should not be a primary concern. It is more important to have logical flow, easy to read/understand and without any obvious inefficiencies. Keeping those within the primary focus will help keep the code length down to a reasonable level.

Once the code is far enough along in development, you can profile it to find the inefficiencies and then benchmark different methods to improve those inefficiencies.

Please put your code within the "code tags" whenever you post any code. The code tags will retain the indentation which will make it easier for us to read and follow your code logic.

Here are a few code comments for you to consider.

All Perl scripts you write should include the strict and warnings pragmas. Those pragmas will aide in pointing out lots of common coding mistakes which can be difficult to track down as the code expands. So, begin all scripts like this:


use strict;
use warnings;

The strict pragma requires you to declare your vars, which is normally done with the 'my' keyword which declares a lexical var or you can use the 'our' keyword to declare a global var if required. You vary rarely need to use global vars.


open FP, "$path/Analysis.txt";

That line has multiple problems.
1) You should ALWAYS check the return code of an open call to verify that it was successful and take proper action if it failed instead of blindly assuming that it succeeded.

2) It is better/safer to use the 3 arg form of open especially if any portion of it was provided by user input.

3) It is best practice to use a lexical var for the file handle instead of a bareword.

open my $analysis_fh, '<', "$path/Analysis.txt" or die "failed to open '$path/Analysis.txt' <$!>";



That statement is not doing exactly what you think. The condition that it's testing is the return value of chomp, not the assignment of $r. The issue here is that it will needlessly generate the following warning if the last line of the file does not contain a line terminator.

Use of uninitialized value in chomp at ...


for ( $i = 0 ; $i < scalar @{ $pp_tot{$lf_cd}{$yrmo} } ; $i++ ) {

Perl's for loop syntax is cleaner and more efficient than the C style loop you're using.

for my $i ( 0..$#{ $pp_tot{$lf_cd}{$yrmo} } ) {

I might even break it up slightly for additional clarity.

my $last_index = $#{ $pp_tot{$lf_cd}{$yrmo} }; 
for my $i (0..$last_index) {

(This post was edited by FishMonger on Jan 1, 2013, 8:09 AM)

Edit Log:
Post edited by FishMonger (Veteran) on Jan 1, 2013, 8:09 AM

Search for (options) Powered by Gossamer Forum v.1.2.0

Web Applications & Managed Hosting Powered by Gossamer Threads
Visit our Mailing List Archives