Re: Please help critcize and shorten my sub code

[Richard Lee]

Jenda Krynicky wrote:
From:                   Richard Lee <[EMAIL PROTECTED]>
  
I dont know how to go through the array over and over again pending on 
my previous search so I ended up writing it like below which works.. but 
looks really really
inefficient..

sub dd_fact {
     my $routename = shift;
     my $routegroupid;
     my $trunkgroupid;
     my $carriername;
     my $carrier_active;
     my $carrierid;
    

You can declare several variables at once:

     my ($routegroupid, $trunkgroupid, $carriername, $carrier_active, 
$carrierid);

  
     AHI: for (@dat) {
        if (exists $_->{outsideroute_group_m}{route_name}
             and $_->{outsideroute_group_m}{route_name} eq "$routename") {
             $routegroupid = $_->{outsideroute_group_m}{route_group_id};
             last AHI;
        }
     }
    

You do not need the label unless you need to jump out of some other 
loop than the innermost. In this case there are no nested loops so it 
would probably be better to skip the label.

Also you do not need to, and should not, enclose variables in quotes. 
This is Perl, not shell scripting.

  
     EWF: for (@dat) {
...
    

You may do something like this:

  for my $wanted ( 
      [outsideroute_group_m => route_name => \$routename, 
route_group_id => \$routegroupid], 
      [outsideroute_trunk_m => route_group_id => \$routegroupid, 
trunkgroup_id => \$trunkgroupid], 
      [outsideotrunkgroup_m => trunkgroup_id => \$trunkgroupid, 
carrier_id => \$carrierid], 
      [outsidecarrier_m => carrier_id => \$carrierid, carrier_name => 
\$carriername], 
      [outsidecarrier_m => carrier_id => \$carrierid, active => 
\$carrier_active], 
  ) {
      if (exists $_->{$wanted->[0]}{$wanted->[1]} 
        and $_->{$wanted->[0]}{$wanted->[1]} eq ${$wanted->[2]}) {
          ${$wanted->[4]} = $_->{$wanted->[0]}{$wanted->[3]};
          last;
      }
  }

  return($trunkgroupid,$carriername,$carrier_active);
}

But I think you should rather rethink and rework your data structure. 
Or possibly keep the data in a database (DBD::SQLite would probably 
be best, no need for any external application, the whole database 
engine in built into the module) and you can query the data to your 
hearts content.

Jenda
===== [EMAIL PROTECTED] === http://Jenda.Krynicky.cz =====
When it comes to wine, women and song, wizards are allowed 
to get drunk and croon as much as they like.
        -- Terry Pratchett in Sourcery


  
thanks.. I am looking at your solution(looks very complicated) and also 
will alos look into DBD::SQLite as well

thanks a bunch for quick response.

--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
http://learn.perl.org/