Rather than all those individually named totals and counts, use a hash to store them and a sub to make totalling them generic. Just add more osnames to the initialisation of the hash, and you never need change the rest of the program when a new one comes along.

sub logtime {
    my( $oref, $freecall, $voicemail, $osname, $dur, $bnum ) = @_;
    
    $oref->{ totaldur }{ $osname } += $dur;
    $oref->{ totalcdr }{ $osname }++;
    if( defined $freecall->{ $bnum } ) {
        $oref->{ freedur }{ $osname } += $dur;
        $oref->{ freecdr }{ $osname }++;
    }
    elsif( defined $voicemail->{ $bnum } ) {
        $oref->{ voicedur }{ $osname } += $dur;
        $oref->{ voicedcr }{ $osname }++;
    }
}

my( %o, %freecall, %voicemail, $osname, $dur, $bnum );

# Init with all known osnames.
@o{ qw[ INTIM TOUCH ... ] } = (); 

if ( defined $osname{$num1} 
and exists $o{ $osname{ $num1 } } 
) { 
    logtime( \%o, \%freecall, \%voicemail
    , $osname{ $num1 }, $dur, $bnum );
}
else {
    warn 'Undefined or unknown osname';
}
[download]

Note: This isn't meant to be working code, just a starting point. You could/should? probably combine the freecall and voicemail hashes into one.

my %calls = (
    voice => {
       123 => undef, 
       789 => undef,
    ],
    free => {
       456 => undef,
    ],
);
[download]

If they are mutually exclusive, you could key the combined hash the other way

my %calls = ( ## Keys are possible $bnum values
   123 => 'voice',
   456 => 'free',
   789 => 'voice',
   ...
);
[download]

Hi browserUK,

Nothing to do, that my simple mistake and I'm had changed that.

But, I've one question here. How I can moved the next variable into next line and passed to the sub routine, as example ?


     logitme ($rec, $calltype, $num1, $num2, $anum, $bnum, $cnum, $dur
+, $intrunk, $freecall, $sms, $operatorTdb, $routehdb, $voicemail, $op
+eratorGrp
);
[download]

That is very long variable, I have tried like this :

     logitme($rec, $calltype, $num1, $num2, /
             $anum, $bnum, $cnum, $dur, $intrunk, /
             $freecall, $sms, $operatorTdb, /
             $routehdb, $voicemail, $operatorGrp);
[download]

But it's failed and come out with some error (as my previous mail). could you help me ?

Thanks,
bh_perl

Just drop the slashes. I guess you meant backslashes, but they are not needed here.

Hi BrowserUK,
Thanks for your good advise,
Once I'm followed your nice ideas, some error found here which are :


         Not a HASH reference at test.pl line ?? .....
[download]

What does means ?, or I'm missing something like use strict or ???

The error line refer to the "freecall" hash

Hopefully, my text format is OK right now ;-)

I think that a table-driven approach might make the logic of what you're doing more evident:

my @cases = (
  { 
    osname => 'INTIM', 
    total => {
      duration => \$ototalintimdur, counter => \$ototalintimcdr.
    },
    freecall => {
      duration => \$ointimfreedur,  counter => \$ointimfreecdr.
    },
    voicemail => {
      duration => \$ointimvoicedur, counter => \$ointimvoicecdr.
    },
  },
  { 
    osname => 'TOUCH', 
    total => {
      duration => \$ototaltouchdur, counter => \$ototaltouchcdr.
    },
    freecall => {
      duration => \$otouchfreedur,  counter => \$ototaltouchcdr.
    },
    voicemail => {
      duration => \$otouchvoicedur, counter => \$otouchvoicecdr.
    },
  },
)

if ( defined $osname{$num1} ) {
  foreach my $case ( @cases ) { 
    next unless $osname{$num1} =~ /$case->{osname}/;
    my $calltype = ( defined $freecall{$bnum}  ) ? 'freecall'  :
           ( defined $voicemail{$bnum} ) ? 'voicemail' : 
                            undef;
    my @counter_sets = map $case->{$_}, grep $_, 'total', $calltype;
    foreach my $counters ( @counter_sets ) {
      ${ $counters->{duration} } += $dur;
      ${ $counters->{counter}  } ++;
    }
    last;
  }
}
[download]