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

While writing below program few problems 1)so far it's doing what it's suppose to do ;
but I am sure it can be optimized..(rather, I am sure there are lot of mistakes) can someone please advise
2)I am not sure how to access inside of @data1, for example, how do I get total total number of %ipstn_s ?(beside doing ++ ), I thought I could just assign to scalar but ...
use strict; use diagnostics; use Text::CSV_XS; use Data::Dumper; my ($counter,$ncounter); my (@data , @data1); my @attrs_sto = qw( _status _gwname _acct_id _call_duration _eoc_reason _call_direction _called_num _i_pstn_trunk _i_pstn_circuit _e_pstn_circuit _e_pstn_trunk ); my @attrs_att = qw( _status _gwname _acct_id _eoc_reason _call_direction _called_num _i_pstn_trunk _i_pstn_circuit _e_pstn_circuit _e_pstn_trunk ); my @attrs_sta = qw( _status _gwname _acct_id _call_direction _called_num _i_pstn_trunk _i_pstn_circuit _e_pstn_circuit _e_pstn_trunk ); @ARGV = ('./sonuslog') unless @ARGV; my $csv = Text::CSV_XS->new; while (<>) { chomp; $csv->parse($_); my @fields = $csv->fields; my %rec; if (/^STOP/) { @rec{@attrs_sto} = @fields[0,1,2,13,14,16,20,33,34,36,67 +]; } elsif (/^START/) { @rec{@attrs_sta} = @fields[0,1,2,11,15,28,29,31,53]; } else { @rec{@attrs_att} = @fields[0,1,2,11,13,17,30,31,33,57]; } push @data, \%rec; } my %ipstn; foreach (@data) { my %ipstn_s; my %ipstn_a; $counter++; if ($_->{_status} eq 'STOP') { next ; } else { if ($_->{_status} eq 'START' && $_->{_i_pstn_trunk}) { my $i_pstn_si = extract($_->{_i_pstn_circuit}, $_->{_i_p +stn_trunk}); push @{$ipstn_s{$i_pstn_si}}, $_; pushnow(\%ipstn_s); } else { if ($_->{_status} eq 'START') { my $e_pstn_se = extract($_->{_e_pstn_circuit}, $_->{ +_e_pstn_trunk}); push @{$ipstn_s{$e_pstn_se}}, $_; pushnow(\%ipstn_s); } else { if ($_->{_status} eq 'ATTEMPT' && $_->{_i_pstn_trunk} +) { my $i_pstn_ai = extract($_->{_i_pstn_circuit}, $_- +>{_i_pstn_trunk}); push @{$ipstn_a{$i_pstn_ai}}, $_; pushnow(\%ipstn_a); } else { if ($_->{_status} eq 'ATTEMPT') { my $e_pstn_ae = extract($_->{_e_pstn_circuit}, +$_->{_e_pstn_trunk}); push @{$ipstn_a{$e_pstn_ae}}, $_; pushnow(\%ipstn_a); } else { $ncounter++; } } } } } } print Dumper(@data1); sub pushnow { push @data1, @_; } sub extract { return join('-', (split(/:/, $_[0]))[1,2], $_[1]); }
example of output from above
$VAR14029 = { '11-57-USNxxxxxxxxxx' => [ { '_acct_id' => '0xxxxxxxxxx +xxxxxx', '_i_pstn_trunk' => '', '_called_num' => '5551213' +, '_status' => 'START', '_gwname' => 'xxxxxxxxxxxx +', '_i_pstn_circuit' => '', '_e_pstn_circuit' => '1:11 +:57:8:0:0x00000000:0x00000000', '_e_pstn_trunk' => 'USNxxx +xxxxxxxxx', '_call_direction' => 'IP-T +O-PSTN' } ] }; $VAR14030 = { '10-12-USNxxxxxxxx_D' => [ { '_acct_id' => '0xxxxxxxxxxx +xxxxxxxxx', '_i_pstn_trunk' => 'USNxxxx +xxxx_D', '_called_num' => '5551212', '_status' => 'START', '_gwname' => 'xxxxxxxxxxx', '_i_pstn_circuit' => '1:10: +12:22:8350:0x0001F8F0:0x00056940', '_e_pstn_circuit' => '', '_e_pstn_trunk' => '', '_call_direction' => 'PSTN- +TO-IP' } ] };

Replies are listed 'Best First'.
Re: help on my overall program
by GrandFather (Saint) on Oct 15, 2007 at 03:40 UTC

    For a start the horrible nested if/else if code in the for loop can be cleaned up by using elsif (which you actually use in the while loop!):

    foreach (@data) { my %ipstn_s; my %ipstn_a; next if $_->{_status} eq 'STOP'; if ( $_->{_status} eq 'START' && $_->{_i_pstn_trunk} ) { my $i_pstn_si = extract( $_->{_i_pstn_circuit}, $_->{_i_pstn_t +runk} ); push @{ $ipstn_s{$i_pstn_si} }, $_; pushnow( \%ipstn_s ); } elsif ( $_->{_status} eq 'START' ) { my $e_pstn_se = extract( $_->{_e_pstn_circuit}, $_->{_e_pstn_t +runk} ); ... pushnow( \%ipstn_a ); } else { $ncounter++; } }

    Note too that $counter has been removed - it should be identical with @data used in scalar context. Also the next is handled by an if modifier which removes another level of indentation.


    Perl is environmentally friendly - it saves trees