>       >>>>> "CS" == Chris Stinemetz <cstinem...@cricketcommunications.com> 
> writes:

>       CS> Thanks Chris
>       CS> I am trying to build a sub routine that computes the following 
> equation

>       CS> When $data[262] has the value of 1
>       CS> Take the value of $data[261] and return the solution of: 
> $data[261]+13)/6.6/8/2*10)/10

>       CS> When I call the sub routine before I push the array I would like
>       CS> to return the value in 0.1 decimal.

>       that makes no sense. a floating number is just what it is. only when
>       printing it does counting decimal places make sense.

>       CS> use warnings;
>       CS> use strict;

>       CS> my $filepath = 'C:/temp/PCMD';
>       CS> my $outfile  = 'output.txt';

>       CS> open my $fh, '<', $filepath or die "ERROR opening $filepath: $!";
>       CS> open my $out, '>', $outfile or die "ERROR opening $outfile: $!";

>       CS> my @array = ();

>       CS> while (<$fh>){

>       better to assign the line to a variable. using $_ all the time isn't a
>       great idea for several reasons. the topmost one is that named variables
>       make the program more readable.

>       CS>                 next unless /;/;
>       CS>                 chomp;
>       CS>     my @data = split /;/;
>       CS>     my($cell,$sect,$chan,$carr,$dist,$precis) = 
> ($data[31],$data[32],$data[38],$data[39],$data[261],$data[262]);

>       use a slice to save typing and eyestrain. you can access multiple
>       array/hash elements at one time:

>       my($cell,$sect,$chan,$carr,$dist,$precis) = @data[31,32,38,39,261,262];



>       CS>                                 if ($data[38] == 15)

>       why are you using @data again when you just assigned 38 to $chan? use
>       the scalar so it makes more sense to the reader.

>       rule: always write code so the reader will understand it.

>       CS>                                 {
>       CS>                                                 $data[39] = 2;
>       CS>                                 } else {
>       CS>                                                 $data[39] = 1;
>       CS>                                 }

>       again, use the scalar. and i don't see $data[39] used anywhere else so
>       that assignment is lost. also use the conditional expression. that whole
>       thing reduces to this:

>       $carr = ( $chan == 15 ) ? 2 : 1 ;

>       also since you are overwriting $chan there, you don't need it in the
>       slice above. $carr can be declared right here too.



>       CS>                                                 if( 
> length($data[261]) > 12)
>       CS>                                                 {
>       CS>                                                                 
> $dist = getDist;
>       CS>                                                 }
>       CS>                                                 else
>       CS>                                                 {
>       CS>                                                                 
> $dist = "";
>       CS>                                                 }

>       ditto here.

>       my $dist = ( length( $dist ) > 1 ) ? getDist() : '' ;


>       CS>                 push @array, {

>       don't name arrays, @array. pick a name that describes the data in it.

>       CS>                                 cell => $cell,
>       CS>                                 sect => $sect,
>       CS>                                 carr => $carr,
>       CS>                                 RTD  => $RTD,

>       where does $RTD come from? i don't see it anywhere else so it will fail
>       under strict.

>       CS>                 };
>       CS> }

>       CS> my @sorted = map { $_->[0] }
>       CS>               sort {
>       CS>                  $a->[1] <=> $b->[1]
>       CS>                  || $a->[2] <=> $b->[2]
>       CS>                  || $a->[3] <=> $b->[3]
>       CS>               }
>       CS>               map { [ $_, $_->{cell}, $_->{sect}, $_->{carr} ]}
>       CS>               @array;

>       gack. i won't even address that now. but look at Sort::Maker on cpan for
>       an easier and more descriptive way to sort complex things.

>       also since you already have the values in a hash, there is no reason to
>       assign them to an array in there. 


>       CS> for my $tuple ( @sorted ){
>       CS>     print "$tuple->{cell} $tuple->{sect} $tuple->{carr} 
> $tuple->{RTD}\n";
>       CS>                 }

>       slice to the rescue again:

>       print join( ' ', @tuple{ qw( cell sect carr RTD ), "\n" ;


>       CS> for my $tuple ( @sorted ){
>       CS>     print $out "$tuple->{cell} $tuple->{sect} $tuple->{carr} 
> $tuple->{RTD}\n";
>       CS>                 }
>       CS>                 close $out;

>       but why print the same thing twice? store the string in a var, and then
>       print to both places. or even better, use File::Slurp and simplify.

>       here is a complete rewrite (untested):


>       use warnings;
>       use strict;

>       use File::Slurp ;

>       my $filepath = 'C:/temp/PCMD';
>       my $outfile  = 'output.txt';

>       my %cols = (

>       cell  => 31,
>       sect  => 32,
>       chan  => 38,
>       dist  => 261,
>       precis      => 262,
>       ) ;

>       my @records ;

>       my @lines = read_file( $filepath ) ;

>       chomp @lines ;

>       foreach my $line ( @lines ) {

>       next unless $line =~ /;/ ;

>       my %record ;

>       # this gets just what you want into a hash using a hash slice and an
>       # array slice. the order of keys and values will be the same for any
>       # given hash

>       @record{ keys %cols } = (split /;/, $line)[ values %cols ] ;

>       $record{carr} = ( $record{chan} == 15 ) ? 2 : 1 ;     
>       $record{dist} = ( length( $record{dist}) > 1 ) ? getDist() : '' ;

>       push( @records, $record ) ;
>       }

>       my @sorted = sort {
>       $a->{cell} <=> $b->{cell} ||
>       $a->{sect} <=> $b->{sect} ||
>       $a->{carr} <=> $b->{carr}
>       } @records ;


>       my @report = map "@{$_{ keys %cols }}\n", @records ;

>       print @report ;
>       write_file( $output, @report ) ;


>       now isn't that a lot cleaner? some parts (all the slicing) may take a
>       little understanding but that isn't too hard.

>       uri

>       -- 
>       Uri Guttman  ------  u...@stemsystems.com  --------  
> http://www.sysarch.com --
>       -----  Perl Code Review , Architecture, Development, Training, Support 
> ------
>       ---------  Gourmet Hot Cocoa Mix  ----  http://bestfriendscocoa.com 
> ---------

Thanks for the explanations. This really cleaned up my code. Do you think you 
could explain to me how to get 
my dist sub routine to work correctly? I am very new to perl so please forgive 
my ignorance. 

I am getting the following errors.

syntax error at ./DOband1.pl line 12, near "$record{dist"
syntax error at ./DOband1.pl line 12, near "13)"
Execution of ./DOband1.pl aborted due to compilation errors.

My updated code is on the bottom.

#!/usr/bin/perl

use warnings;
use strict;

use File::Slurp;

my $filepath = 'C:/temp/PCMD';
my $output  = 'output.txt';

sub Dist {
      my $record{dist}+13)/6.6/8/2*10)/10 # I'm am not sure how to handle this. 
      }

my %cols = (
      cell  => 31,
      sect  => 32,
      chan  => 38,
      dist  => 261,
      precis      => 262,
);

my @records;

my @lines = read_file( $filepath );

chomp @lines;

foreach my $line ( @lines ) {

      next unless $line =~ /;/;

      my %record;

# this gets just what you want into a hash using a hash slice and an # array 
slice. 
# the order of keys and values will be the same for any # given hash

      @record{ keys %cols } = (split /;/, $line)[ values %cols ] ;

      $record{carr} = ( $record{chan} == 15 ) ? 2 : 1 ;     
      $record{dist} = ( length( $record{dist}) > 1 ) ? getDist() : '' ;

      push( @records, my $record ) ;
}

my @sorted = sort {
            $a->{cell} <=> $b->{cell} ||
            $a->{sect} <=> $b->{sect} ||
            $a->{carr} <=> $b->{carr}
      } @records ;


my @report = map "@{$_{ keys %cols }}\n", @records ;

print @report ;
write_file($output, @report) ;

--
To unsubscribe, e-mail: beginners-unsubscr...@perl.org
For additional commands, e-mail: beginners-h...@perl.org
http://learn.perl.org/


Reply via email to