stockroomguy has asked for the wisdom of the Perl Monks concerning the following question:

I'm using a pair of arrays containing anonymous hashes to add URLs to some Item ID's for our stockroom's online catalog. To do this, I use the following code to add a key/value pair to those anonymous hashes:

$n=0; for my $el (@products) { if ( exists $products[$n]{"Vendor Item ID"} ) { $m = 0; for my $el (@config) { if ( exists $config[$m]{Vendor} ) { if ( $config[$m]{Vendor} =~ /$products[$n]{Name}/ ) { $url= $config[$m]{URL}; $url=~ s/\*/$products[$n]{"Vendor Item ID"}/g; $products[$n]{TaggedID} = $url; } } $m++; } } $n++; }

This works fine--as long as I don't use strict. The problem is that if I use strict, and declare the $url variable with 'my', running that substitution seems to clear out the variable, so that $products$n{TaggedID} gets set to nothing.

Why? And on a related note, is there a better way to do this? I'm still pretty new to perl.

For reference, one of the URLs looks like this: http://www1.qiagen.com/Search/Search.aspx?SearchTerm=*and the idea is to trade the * for whatever is in the 'Vendor Item ID' column, then store that in the appropriate hash.

Replies are listed 'Best First'.
Re: Why does this code break if I properly set the scope?
by Khen1950fx (Canon) on Oct 11, 2009 at 05:47 UTC
    I tried this. Maybe it'll help:

    #!/usr/bin/perl use strict; use warnings; use diagnostics; my $n=0; for my $el (my @products) { if ( exists $products[$n]{'Vendor Item ID'} ) { my $m = 0; for my $el (my @config) { if ( exists $config[$m]{'Vendor'} ) { if ( $config[$m]{'Vendor'} =~ /$products[$n]{Name}/ ) +{ my $url= $config[$m]{'URL'}; $url=~ s/\*/$products[$n]{'Vendor Item ID'}/g; $products[$n]{'TaggedID'} = $url; } } ++$m; } } ++$n; }
Re: Why does this code break if I properly set the scope?
by CountZero (Bishop) on Oct 11, 2009 at 17:02 UTC
    There is something strange with your code.

    You have an array @products which you loop over element by element, storing each element in the lexical variable $el, yet you never use this variable. Rather you use a variable $n to index into @products.

    You do the same with the array @config with another lexical variable $el (which will make the other $el inaccessible) and $m.

    Your loops will become much clearer if you drop the use of the index-variables and directly use the lexical variables.

    These lexical variables are aliases for the elements of the arrays, so any change you make to them will really be made to the actual element of the array.

    I have replaced your if tests by a next unless ... construct. If your arrays are quite regular, i.e. all elements contain an anonymous hash with respectively the 'Vendor Item ID' and 'Vendor' keys alway present, then you can even drop these two next unless ... constructs. Note: you are testing if these keys exist, not whether the value of the key exists or is defined. It could be empty or undef/ Maybe you should be testing for that?

    Another comment concerns the test where you check to see whether the value of the product 'Name'-key matches the value of the 'Vendor' key in the config array. Are you sure you need a regex or would a simple test for equality (== or eq) suffice?

    use strict; use warnings; for my $product (@products) { next unless exists $product->{'Vendor Item ID'}; for my $config_element (@config) { next unless exists $config_element->{'Vendor'}; if ($config_element->{'Vendor'} =~ $product->{'Name'}) { my $url = $config_element->{'URL'}; $url =~ s/\*/$product->{'Vendor Item ID'}/g; $product->{'TaggedID'} = $url; } } }

    CountZero

    Update: Put in proper dereferencing ('->').

    A program should be light and agile, its subroutines connected like a string of pearls. The spirit and intent of the program should be retained throughout. There should be neither too little or too much, neither needless loops nor useless variables, neither lack of structure nor overwhelming rigidity." - The Tao of Programming, 4.1 - Geoffrey James

Re: Why does this code break if I properly set the scope?
by gmargo (Hermit) on Oct 11, 2009 at 17:04 UTC
    I was able to run your code with example data. One potential problem is the line:
    if ( $config[$m]{Vendor} =~ /$products[$n]{Name}/ ) {
    which won't work right if any vendor name is a substring of another. Another potential problem arises if the URL is not defined. Perhaps there are other problems with the input data.

    Here's a version of your code with some example data and some debugging, which works for me:

    #!/usr/bin/perl use strict; use warnings; use diagnostics; my @products = ( { 'Prod' => 'pencil', 'Name' => "Staples", 'Vendor Item ID' => " +1", }, { 'Prod' => 'paper', 'Name' => "OfficeMax", 'Vendor Item ID' => " +2", }, { 'Prod' => 'eraser', 'Name' => "Walgreens", 'Vendor Item ID' => " +3", }, ); my @config = ( { 'Vendor' => "Staples", 'URL' => "http://staples/item=*", }, { 'Vendor' => "OfficeMax", 'URL' => "http://officemax/item=*", }, { 'Vendor' => "Walgreens", 'URL' => "http://walgreens/item=*", }, ); my ($n, $m, $url); print_products("BEFORE"); $n=0; for my $el (@products) { if ( exists $products[$n]{"Vendor Item ID"} ) { $m = 0; for my $el (@config) { if ( exists $config[$m]{Vendor} ) { if ( $config[$m]{Vendor} =~ /$products[$n]{Name}/ ) { $url= $config[$m]{URL}; $url=~ s/\*/$products[$n]{"Vendor Item ID"}/g; $products[$n]{TaggedID} = $url; } } $m++; } } $n++; } print_products("AFTER"); sub print_products { my ($msg) = @_; print "$msg\n" if defined $msg; my $count = 0; foreach my $prod (@products) { $count++; print "$count:"; foreach my $key (sort keys %$prod) { print " \"$key\"=\"".$$prod{$key}."\""; } print "\n"; } }

    And, since you asked for better ways to do it, here is the main loop the way I would have written it:

    foreach my $prod (@products) { next if !exists $$prod{'Vendor Item ID'}; my $viid = $$prod{'Vendor Item ID'}; my $name = $$prod{'Name'}; foreach my $conf (@config) { next if !exists $$conf{'Vendor'}; if ( $$conf{'Vendor'} eq $name ) { next if !exists $$conf{'URL'}; my $url = $$conf{'URL'}; $url =~ s/\*/$viid/; $$prod{'TaggedID'} = $url; last; } } }