Re: Please help critcize and shorten my sub code

Jenda Krynicky wrote:
> From: Richard Lee <[email protected]>
>   
>> Jenda Krynicky wrote:
>>     
>>> 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);
>>> }
>>>   
>>>       
>> thanks.. I am looking at your solution(looks very complicated) and also 
>> will alos look into DBD::SQLite as well
>>     
>
> It's not terribly complicated once you get used to references. I just 
> extracted the differing parts from your loops so that I could replace 
> them by a single one. 
>
> And I made a mistake doing so, the code should have been 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], 
>   ) {
>     for (@dat) {
>       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);
> }
>
> I've forgotten the inner loop.
>
> Maybe you'd like it better if I would have hidden the inner loop in a 
> subroutine:
>
>   my $routegroupid = findValue( 'outsideroute_group_m', 'route_name', 
> $routename, 'route_group_id'); 
>   my $trunkgroupid = findValue( 'outsideroute_trunk_m', 
> 'route_group_id', $routegroupid, 'trunkgroup_id'); 
>   my $carrierid = findValue( 'outsideotrunkgroup_m', 'trunkgroup_id', 
> $trunkgroupid, 'carrier_id'); 
>   my $carriername = findValue( 'outsidecarrier_m', 'carrier_id', 
> $carrierid, 'carrier_name'); 
>   my $carrier_active = findValue( 'outsidecarrier_m', 'carrier_id', 
> $carrierid, 'active'); 
>
>  return($trunkgroupid,$carriername,$carrier_active);
> }
>
> sub findValue {
>   my ($dat, $section, $lookup_key, $lookup_value, $value_key) = @_;
>   for (@$dat) {
>     if (exists $_->{$section}{$lookup_key}
>      and $_->{$section}{$lookup_key} eq $lookup_value) {
>       return $_->{$section}{$value_key};
>     }
>   }
> }
>
> Looking at it again, I would like it better as well.
>
> Jenda
> P.S.: The => (sometimes called "fat comma") is almost equivalent to 
> an ordinary comma, the only difference is that if the thing to the 
> left of the => looks like a word it's automaticaly quoted. So
>   [one => two => $three]
> is equivalent to
>   ['one', 'two', $three]
>
> It's just syntactic sugar.
> ===== [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
>
>
>   
thank you.

this is excellent and this is what I need to start to think more in 
terms of...
re-using the code.. or rather finding ways to re-use the code.

thank you!!! excellent job!