Re: hash

Bobby wrote:
> Here's a better excerpt of the script, see my comment towards the bottom. Thanks. 
> 
> #!/usr/local/bin/perl

use warnings;

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

You should include the $! variable in the error message so you know 
*why* open failed.

> my %US;
> my %EURO;
> my %ITAL;
> my %GENERAL;
> 
> my $DEBUG = 0;
> 
> # map each store to its children stores
> while (<IN>)
> {
>     chomp;
>     my $line = $_;
>     # split rows into columns
>     my @fields = split /\|/, $line;

Why assign $_ to $line and not just use $_ throughout?

You are only using five fields so:

     my ( $sku, $item_size, $standard_size, $size_type, $us_sizes ) = ( 
split /\|/, $line )[ 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";
>     }
> 
>     # make sure base number field is numeric
>     if($fields[0] =~ /^[0-9]{5}/) {
>         # 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];
>                 
>         next if($item_size eq ""); #if item size is note specified, skip it.
> 
>         # 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 "") {

This test is unnecessary as foreach will not iterate over an empty list.

>             $us_sizes =~ s/^,?(.*),?$/\1/g;

You don't need to remove the trailing comma as split() will not return 
empty trailing fields.  \1 should only be used inside a regular 
expression, in the replacement string you should use $1 instead.  The /g 
option is superfluous as the pattern is anchored both at the beginning 
and the end.

>             foreach my $size (split /,/, $us_sizes) {
>                 #print "pushing found US size $size\n";
>                 $US{$size} = 1;
>             }
>         }

         foreach my $size ( split /,/, $us_sizes ) {
             next unless length $size;
             #print "pushing found US size $size\n";
             $US{ $size } = 1;
         }

>     }
> 
> }
> close(IN);
> 
> 
> printSizesXML();
> 
> sub xmlEscape
> {
>     # STP: Change made to the Ampersand section, to &amp; 
>     my ($str) = @_;
>     $str =~ s/\&/\&amp;/g;
>     $str =~ s/</\&lt;/g;
>     $str =~ s/>/\&gt;/g;
>         
>     #remove weird Unicode character
>     $str =~ s/\x10/ /g;

"\x10" is only a Unicode character if the string in $str is a Unicode 
string.  Usually it is an ASCII control (DLE) character.  If you are 
just replacing one character with another character it would be more 
efficient to use tr/// instead:

     $str =~ tr/\x10/ /;

>     return $str;
> }
> 
> sub in
> {
>     my ($val, @list) = @_;
>     return (grep( /^$val$/, @list));

/^$val$/ is usually written as $_ eq $val.

     return grep $_ eq $val, @list;

> }
> 
> sub printSizesXML
> {
>     my $level = 3;
>     my $spacer = " ";
> 
> 
>     my $n = 1;
>     foreach my $size ( sort keys %US )
>     {
>         next if($size !~ /^[0-9.]+$/);

You are testing that $size only contains '0', '1', '2', '3', '4', '5', 
'6', '7', '8', '9' and/or '.'.

>         $size = xmlEscape($size);

So then how can $size contain '&', '<', '>' or "\x10"?  This function 
call is superfluous.

>         $n++
>     }
>     
>     my $n = 1;

You are defining $n a second time in the same scope as the previous $n. 
  And you don't even use the current value of $n so why is it being 
incremented in the previous loop?

>     foreach my $size ( keys %EURO )
>     {
>         next if($size !~ /^[0-9.]+$/);
>         $size = xmlEscape($size);

Same as before, you have a useless function call.

>         print "$size|Euro\n";
> 
>     
> 
>     #This is where i'm stuck
>     #I need to be able to print the  $us_sizes from $fields[38] 
>     #in the while loop above down in this foreach statement.
>     # The commented out code below is my failed attempt. Need some help here
>     #foreach my $sizeb ( sort keys %US ){
>     #    if ($US{$sku} = $EURO{$sku}){

= is the assignment operator so you are assigning the value of 
$EURO{$sku} to the variable $US{$sku}.

>     #    print "$size Euro - $sizeb US\n";
>     #    }
>     # }
>         $n++
>     }


John
-- 
Perl isn't a toolbox, but a small machine shop where you
can special-order certain sorts of tools at low cost and
in short order.                            -- Larry Wall