Re: hash

[email protected] (Rob Dixon)
Newsgroups perl.beginners
Message-ID <[email protected]>
Bobby wrote:
> 
> Here's a better excerpt of the script, see my comment towards the bottom. Thanks. 
> 
> #!/usr/local/bin/perl
> 
> use strict;

Always

  use warnings;

as well, unless you know exactly what it does and have a good reason not to.

> open(IN, $ARGV[0]) || die "Could not open data file: $ARGV[0]\n";

If you are reading from files specified on the command line, then it is much
easier to simply read from the ARGV file handle.

> my %US;
> my %EURO;
> my %ITAL;
> my %GENERAL;

It is most common to reserve upper case for constant names and lower case for
variable names.

> my $DEBUG = 0;

use constant DEBUG => 0;

> # map each store to its children stores
> while (<IN>)
> {
>     my $line = $_;

while (my $line = <ARGV>) {

>     # split rows into columns
>     my @fields = split /\|/, $line;

  my ($sku, $item_size, $standard_size, $size_type, $us_sizes) =
      @fields[0, 19, 36, 37, 38];

>     if($DEBUG) {
>         print "$line\n";
>         print "SKU:".$fields[0]."\n";
>         print "  item_size ".$fields[19]."\n";
>         print "  standard_size ".$fields[36]."\n";
>         print "  us_sizes ".$fields[37]."\n";
>         print "  us_sizes ".$fields[38]."\n";
>     }

  if (DEBUG) {
print <<FIELDS;
SKU:$sku
  item_size $item_size
  standard_size $standard_size
  us_sizes $size_type
  us_sizes $us_sizes
FIELDS
  }

(I guess the 'us_sizes' label is wrong in the first instance?)

>     # make sure base number field is numeric
>     if($fields[0] =~ /^[0-9]{5}/) {

That test checks whether $fields[0] starts with at least five decimal digits. A
value of '00000ABC??' will pass the test.

To make sure $sku is numeric, you can write

  unless ($sku =~ /\D/) {
    :
  }

>         # save the important fields
>         my $sku = $fields[0];
>         my $item_size = $fields[19];
>         my $standard_size = $fields[36];
>         my $size_type = $fields[37];
>         my $us_sizes  = $fields[38];

Already done.

>         next if($item_size eq ""); #if item size is note specified, skip it.

  next unless $item_size;

(unless you're expecting an item size of zero?)

>         # push each store into array associated with its parent store
>         if($size_type eq "") {
>             $US{$item_size} = 1;
>         }
>         elsif($size_type eq "Euro") {
>             $EURO{$standard_size} = 1;
>             $EURO{$sku} => $sku;
>         }
>         elsif($size_type eq "Italian") {
>             $ITAL{$standard_size} = 1;
>         }
>         elsif($size_type eq "General") {
>             $GENERAL{$standard_size} = 1;
>         }
>         if($us_sizes ne "") {
>             $us_sizes =~ s/^,?(.*),?$/\1/g;
>             foreach my $size (split /,/, $us_sizes) {
>                 #print "pushing found US size $size\n";
>                 $US{$size} = 1;
>             }
>                                 
>         }

I don't want to try to guess what you're doing here. The PID of your previous
posts has vanished, and you are populating one of several hashes according to
the value of a single input field, leaving the others hashes empty I am fairly
certain that you need to draw all the data into a single hash, with the value of
a field called SIZE_TYPE indicating how the rest of the fields are to be
interpreted and processed.

>     }
> 
> }
> close(IN);

In general there's no need to close input files unless you have more than a
couple of them. If you're reading only from ARGV then

[snip subroutines]


I hope this helps.

Rob
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.