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

Hi Monks!
I included a sample of this code I am working on to explain my concerns about the code. I am wondering if there is a more elegant and efficient way of doing what this sample code is showing.
Reason are the values in @locations that can be larger, and all the "IFs", they will be a very large array of code that could be hard to maintain every time one more element is added to this array @locations.
I was thinking building a hash with all possible $locs (see code) for each element in @locations and do the check using that. My problem is I am lost doing that.
Can any one help or suggest a more efficient way of doing this?
Thanks for the Help!
Here is the sample code for this:
#!/usr/bin/perl -w

use strict;

print "Content-type: text/html\n\n";

my @locations = qw(LA LB LC LD LE); # this list could have more values
+, thats my concern with the rest of the code.
my $c_position;

foreach my $location(@locations) {

# Let me explain this value,$test_loc will have a different value for 
+each element of @locations, 
#  I just added this way to show how it would work.
my $test_loc = "Park: SW at 10 position"; #this could be for LA
#my $test_loc = "Park: NE at 21 position"; #this could be for LB
#my $test_loc = "Park: E at 11 position"; #this could be for LC
#my $test_loc = "Park: NW at 17 position"; #this could be for LD
#my $test_loc = "Park: NE at 06 position"; #this could be for LE ...


my $locs = substr $test_loc, 6,2; # maybe using regular expression wou
+ld be more efficient?
   $locs=~s/\s+//;

if( ($location=~/LA/ig) && (
                         ($locs eq "NE") ||
                         ($locs eq "E")  ||
                         ($locs eq "SE") ||
                         ($locs eq "S")  ||
                         ($locs eq "SW")
                        )) {

   $c_position=$c_position." 1LA= NE|E|SE|S|SW";   

 }
elsif( ($location=~/LB/ig) &&  (
                             ($locs eq "NE") ||
                             ($locs eq "SW") ||
                             ($locs eq "W")  ||
                             ($locs eq "NW")
                            )) {

   $c_position=$c_position." 2LB= NE|SW|W|NW";   

 }
elsif( ($location=~/LC/ig) && (
                            ($locs eq "SE") || 
                            ($locs eq "S")  ||
                            ($locs eq "SW")
                           )) {

   $c_position=$c_position." 3LC= SE|S|SW";   

 } 
elsif( ($location=~/LD/ig) && (
                           ($locs eq "SE") || 
                           ($locs eq "S")  ||
                           ($locs eq "SW")
                           )) {

   $c_position=$c_position." 4LD= SE|S|SW";   

 }  
else {
   
   $c_position=$c_position." 5ALL OTHERS HERE.";
 }

} 
print "Results= $c_position\n\n";
exit;

[download]

Replies are listed 'Best First'.
Re: Code Efficiency and Dynamic Help!
by moritz (Cardinal) on Sep 24, 2010 at 14:28 UTC
    Here's a shorter, data-driven version of your program that produces the same output as yours, and should be more efficient too: 
    #!/usr/bin/perl -w
use strict;
use warnings;

print "Content-type: text/html\n\n";

my @locations = qw(LA LB LC LD LE);


my %loc = (
    LA  => { NE => 1, E  => 1, SE => 1, S  => 1, SW => 1 },
    LB  => { NE => 1, SW => 1, W  => 1, NW => 1},
    LC  => { SE => 1, S  => 1, SW  => 1},
    LD  => { SE => 1, S  => 1, SW => 1},
);

my %results = (
    LA => '1LA= NE|E|SE|S|SW',
    LB => '2LB= NE|SW|W|NW',
    LC => '3LC= SE|S|SW',
    LD => '4LD= SE|S|SW',
);

my $c_position = '';

foreach my $location (@locations) {
    my $test_loc = "Park: SW at 10 position"; #this could be for LA
    my $locs = substr $test_loc, 6, 2;
    $locs=~s/\s+//;

    $c_position .= ' ';
    if ($loc{$location}{$locs}) {
        $c_position .= $results{$location};
    } else {
        $c_position .= '5ALL OTHERS HERE.';
    }
}
print "Results= $c_position\n\n";

    [download]

    This is not quite perfect; for example one could unify %results %loc (pretty easy if the order of letters in the values of %results doesn't matter), but it's a start.

    It also makes it possible to read the hashes from config files, so that a long list of conditions requires no code change.

    Perl 6 - links to (nearly) everything that is Perl 6.
[reply]
[d/l]
[select]
      Thats a good start for sure, but I can't have the keys in
      my %loc = (
    LA  => { NE => 1, E  => 1, SE => 1, S  => 1, SW => 1 },
    LB  => { NE => 1, SW => 1, W  => 1, NW => 1},
    LC  => { SE => 1, S  => 1, SW  => 1},
    LD  => { SE => 1, S  => 1, SW => 1},
);

      [download]

      been the same in
      my @locations = qw(LA LB LC LD LE);

      [download]

      because they are codes that will have a different meaning; like LA comes back as "LeadAlpha" and it goes for all the other.
      #!/usr/bin/perl -w
use strict;
use warnings;

print "Content-type: text/html\n\n";

my @locations = qw(LAx LBy LCo LDe LEd);


my %loc = (
    LA  => { NE => 1, E  => 1, SE => 1, S  => 1, SW => 1 },
    LB  => { NE => 1, SW => 1, W  => 1, NW => 1},
    LC  => { SE => 1, S  => 1, SW  => 1},
    LD  => { SE => 1, S  => 1, SW => 1},
);

my %results = (
    LA => '1LA= NE|E|SE|S|SW',
    LB => '2LB= NE|SW|W|NW',
    LC => '3LC= SE|S|SW',
    LD => '4LD= SE|S|SW',
);

my $c_position = '';

foreach my $location (@locations) {
    my $test_loc = "Park: SW at 10 position"; #this could be for LA
    my $locs = substr $test_loc, 6, 2;
    $locs=~s/\s+//;


    $c_position .= ' ';
    if ($loc{$location}{$locs}) {
        $c_position .= $results{$location};
    } else {
        $c_position .= ' ALL OTHERS HERE.';
    }




}
print "Results= $c_position\n\n";

      [download]
[reply]
[d/l]
[select]
Re: Code Efficiency and Dynamic Help!
by roboticus (Chancellor) on Sep 24, 2010 at 16:44 UTC

    I haven't tested this or anything, but this is the direction I'd go if I were writing it:

    #!/usr/bin/perl -w
use strict;

print "Content-type: text/html\n\n";

my @locmap = (  
#         Loc   Prefix      NW N NE W E SW S SE
        [ 'LA', '1LA', [ qw{     NE   E SW S SE} ] ],
        [ 'LB', '2LB', [ qw{NW   NE W   SW     } ] ],
        [ 'LC', '3LC', [ qw{            SW S SE} ] ],
        [ 'LD', '4LD', [ qw{            SW S SE} ] ],
);
my $c_position;

foreach my $L (@locmap) {
  my $location = $$L[0]; # LA, LB, etc
  my $out = $$L[1];      # Prefix value to output
  my @locs = @{$$L[2]};  # Compass directions for this location

  my $locs = substr $test_loc, 6,2;
  $locs=~s/\s+//;

   if ( grep { $locs eq $_ } @locs ) {
     $c_position=$c_position. " $out= ".join('|',@locs)
   }
   else {
     $c_position=$c_position." 5ALL OTHERS HERE.";
   }
}
print "Results= $c_position\n\n";

    [download]

    ...roboticus

[reply]
[d/l]

Back to Seekers of Perl Wisdom