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

I have some (huge) files. In that i need to count some events which satisfying my condition. For that I am using all these if else conditions. Can I reduce the code by using any class (OOP) concepts. Here I cant use "sub" because in error sections the @network array, @subscriber array, @system errror is too big to pass to a function. It may slow down or even memory outage sometimes. So, Please show a smart way to achieve the same. Hope you can understand my requirement. I am just incrementing a variable if the condition satisfies. 

foreach my $Intermediate_file (@Intermediate_files)
{
    open (INTER,"$Intermediate_file") or print "cannot open file\n";
    while (my $intermediate_line = <INTER>)
    {
        chomp $intermediate_line;
        my @intermediate_array = split(/\|/,$intermediate_line);
        if ($intermediate_array[12] eq "GSM" && $intermediate_array[9]
+ eq "MSubmit")
        {
            $intermediate_MO_cnt++;
        }
        if ($intermediate_array[12] eq "GSM" && $intermediate_array[9]
+ eq "MSubmit" && $intermediate_array[7] eq "MsgAccepted")
        {
            $total_MO_success++;
        }
        if ($intermediate_array[12] eq "SMPP" && $intermediate_array[9
+] eq "ESubmit")
        {
            $intermediate_AO_cnt++;
        }
        if ($intermediate_array[12] eq "SMPP" && $intermediate_array[9
+] eq "ESubmit" && $intermediate_array[7] eq "MsgAccepted")
        {
            $total_AO_success++;
        }
        if ($intermediate_array[12] eq "GSM" && $intermediate_array[13
+] eq "SMPP")
        {
            $total_AT++;
        }
#---------------------------------------ERROR SECTION-----------------
+------------------------------------------------------------#
        if ($intermediate_array[12] eq "GSM")
        {
            if (any { /^\Q$intermediate_array[11]\E$/ } @subscriber_er
+ror)
            {
                $MO_userdep_error++;
            }
            elsif (any { /^\Q$intermediate_array[11]\E$/ } @network_er
+ror)
            {
                $MO_network_error++;
            }
            elsif (any { /^\Q$intermediate_array[11]\E$/ } @system_err
+or)
            {
                $MO_system_error++;
            }
        }
        if ($intermediate_array[12] eq "SMPP")
        {
            if($intermediate_array[11]=~ /SMSC/)
            {
                $AO_ESMEdep_error++ if any { /^\Q$intermediate_array[1
+1]\E$/ } @subscriber_error;
            }
            elsif (any { /^\Q$intermediate_array[11]\E$/ } @network_er
+ror)
            {
                $AO_network_error++;
            }
            elsif (any { /^\Q$intermediate_array[11]\E$/ } @system_err
+or)
            {
                $AO_system_error++;
            }
        }
#---------------------------------END OF ERROR SECTION----------------
+-------------------------------------------------------------#
    }
}
print "Completed the Imtermediate section....\n";
#-------------------------------------POSTPAID SECTION----------------
+-------------------------------------------------------------#

foreach my $Postpaid_file (@Postpaid_files)
{
    open (POST ,"$Postpaid_file")     or print "cannot open file\n";
    while(my $cdr_post_line = <POST>)
    {
        chomp $cdr_post_line;
        my @cdr_post_array = split(/\|/,$cdr_post_line);
        if ($cdr_post_array[12] eq "GSM" && $cdr_post_array[9] eq "Sub
+mit")
        {
            $cdr_post_MO_cnt++ if any { /^\Q$cdr_post_array[11]\E$/ } 
+@MOresp_error;
        }
        if ($cdr_post_array[12] eq "SMPP" && $cdr_post_array[9] eq "Su
+bmit")
        {
            $cdr_post_AO_cnt++ if any { /^\Q$cdr_post_array[11]\E$/ } 
+@MOresp_error;
        }
        if ($cdr_post_array[12] eq "GSM" && $cdr_post_array[13] eq "GS
+M" && $cdr_post_array[9] eq "Submit")
        {
            $total_MT_P2P++ unless any { /^\Q$cdr_post_array[11]\E$/ }
+ @MOresp_error;
        }
        if ($cdr_post_array[12] eq "GSM" && $cdr_post_array[13] eq "GS
+M" && $cdr_post_array[9] eq "Submit" && $cdr_post_array[7] eq "Delive
+red")
        {
            $total_MT_P2P_success++;
        }
        if ($cdr_post_array[12] eq "SMPP" && $cdr_post_array[13] eq "G
+SM" && $cdr_post_array[9] eq "Submit")
        {
            $A2P_FDA_count++;
        }
        if ($cdr_post_array[12] eq "SMPP" && $cdr_post_array[13] eq "G
+SM" && $cdr_post_array[9] eq "Submit" && $cdr_post_array[11] eq "SMSC
+_no_error")
        {
            $A2P_FDA_success_count++;
        }
        if ($cdr_post_array[12] eq "SMPP" && $cdr_post_array[13] eq "G
+SM" && $cdr_post_array[9] eq "Submit")
        {
            $total_MT_A2P++;
        }
        if ($cdr_post_array[12] eq "SMPP" && $cdr_post_array[13] eq "G
+SM" && $cdr_post_array[9] eq "Submit" && $cdr_post_array[7] eq "Deliv
+ered")
        {
            $total_MT_A2P_success++;
        }
        if ($cdr_post_array[12] eq "GSM" && $cdr_post_array[13] eq "SM
+PP" && $cdr_post_array[7] eq "Delivered")
        {
            $total_AT_success++;
        } 
#----------------------------POST PAID ERROR SECTION------------------
+-----------------------------------------------------------#
        if ($cdr_post_array[12] eq "GSM")
        {
            if (any { /^\Q$cdr_post_array[11]\E$/ } @network_error)
            {
                $MO_network_error++;
            }
            elsif (any { /^\Q$cdr_post_array[11]\E$/ } @subscriber_err
+or)
            {
                $MO_userdep_error++;
            }
            elsif (any { /^\Q$cdr_post_array[11]\E$/ } @system_error)
            {
                $MO_system_error++;
            }
        }
        if ($cdr_post_array[12] eq "SMPP")
        {
            if($cdr_post_array[11]=~ /SMSC/)
            {
                $AO_ESMEdep_error++ if any { /^\Q$cdr_post_array[11]\E
+$/ } @subscriber_error;
            }
            elsif (any { /^\Q$cdr_post_array[11]\E$/ } @network_error)
            {
                $AO_network_error++;
            }
            elsif (any { /^\Q$cdr_post_array[11]\E$/ } @system_error)
            {
                $AO_system_error++;
            }
        }
        if ($cdr_post_array[12] eq "GSM" && $cdr_post_array[13] eq "GS
+M")
        {
            if (any { /^\Q$cdr_post_array[11]\E$/ } @network_error)
            {
                $MT_P2P_network_error++;
            }
            elsif (any { /^\Q$cdr_post_array[11]\E$/ } @subscriber_err
+or)
            {
                $MT_P2P_userdep_error++;
            }
            elsif (any { /^\Q$cdr_post_array[11]\E$/ } @system_error)
            {
                $MT_P2P_system_error++;
            }
        }
        if ($cdr_post_array[12] eq "SMPP" && $cdr_post_array[13] eq "G
+SM")
        {
            if (any { /^\Q$cdr_post_array[11]\E$/ } @network_error)
            {
                $MT_A2P_network_error++;
            }
            elsif (any { /^\Q$cdr_post_array[11]\E$/ } @subscriber_err
+or)
            {
                $MT_A2P_userdep_error++;
            }
            elsif (any { /^\Q$cdr_post_array[11]\E$/ } @system_error)
            {
                $MT_A2P_system_error++;
            }
        }
        if ($cdr_post_array[12] eq "GSM" && $cdr_post_array[13] eq "SM
+PP")
        {
            if (any { /^\Q$cdr_post_array[11]\E$/ } @network_error)
            {
                $AT_network_error++;
            }
            elsif (any { /^\Q$cdr_post_array[11]\E$/ } @subscriber_err
+or)
            {
                $AT_userdep_error++;
            }
            elsif (any { /^\Q$cdr_post_array[11]\E$/ } @system_error)
            {
                $AT_system_error++;
            }
        }
#=====================================Submission failure Post paid sec
+tion===============================================#
        if ($cdr_post_array[11] eq "$db_user_errors[0]")
        {
            $user_first_error++;
        }
        elsif ($cdr_post_array[11] eq "$db_user_errors[1]")
                {
                        $user_second_error++;
                }
        elsif ($cdr_post_array[11] eq "$db_user_errors[2]")
                {
                        $user_third_error++;
                }
        elsif ($cdr_post_array[11] eq "$db_user_errors[3]")
                {
                        $user_forth_error++;
                }
        elsif ($cdr_post_array[11] eq "$db_user_errors[4]")
                {
                        $user_fifth_error++;
                }
        if ($cdr_post_array[11] eq "$db_ESME_errors[0]")
                {
                        $ESME_first_error++;
                }
                elsif ($cdr_post_array[11] eq "$db_ESME_errors[1]")
                {
                        $ESME_second_error++;
                }
                elsif ($cdr_post_array[11] eq "$db_ESME_errors[2]")
                {
                        $ESME_third_error++;
                }
                elsif ($cdr_post_array[11] eq "$db_ESME_errors[3]")
                {
                        $ESME_forth_error++;
                }
                elsif ($cdr_post_array[11] eq "$db_ESME_errors[4]")
                {
                        $ESME_fifth_error++;
                }
           if ($cdr_post_array[11] eq "$db_system_errors[0]")
                {
                        $system_first_error++;
                }
                elsif ($cdr_post_array[11] eq "$db_system_errors[1]")
                {
                        $system_second_error++;
                }
                elsif ($cdr_post_array[11] eq "$db_system_errors[2]")
                {
                        $system_third_error++;
                }
                elsif ($cdr_post_array[11] eq "$db_system_errors[3]")
                {
                        $system_forth_error++;
                }
                elsif ($cdr_post_array[11] eq "$db_system_errors[4]")
                {
                        $system_fifth_error++;
                }    
        if ($cdr_post_array[11] eq "$db_network_errors[0]")
                {
                        $network_first_error++;
                }
                elsif ($cdr_post_array[11] eq "$db_network_errors[1]")
                {
                        $network_second_error++;
                }
                elsif ($cdr_post_array[11] eq "$db_network_errors[2]")
                {
                        $network_third_error++;
                }
                elsif ($cdr_post_array[11] eq "$db_network_errors[3]")
                {
                        $network_forth_error++;
                }
                elsif ($cdr_post_array[11] eq "$db_network_errors[4]")
                {
                        $network_fifth_error++;
                }

#==============================Delivery failure postpaid section======
+=========================================#
        if ($cdr_post_array[11] eq "$del_user_errors[0]")
        {
            $del_user1_error++;
        }
                elsif ($cdr_post_array[11] eq "$del_user_errors[1]")
                {
                        $del_user2_error++;
                }
                elsif ($cdr_post_array[11] eq "$del_user_errors[2]")
                {
                        $del_user3_error++;
                }
                elsif ($cdr_post_array[11] eq "$del_user_errors[3]")
                {
                        $del_user4_error++;
                }
                elsif ($cdr_post_array[11] eq "$del_user_errors[4]")
                {
                        $del_user5_error++;
                }
                if ($cdr_post_array[11] eq "$del_network_errors[0]")
                {
                        $del_network1_error++;
                }
                elsif ($cdr_post_array[11] eq "$del_network_errors[1]"
+)
                {
                        $del_network2_error++;
                }
                elsif ($cdr_post_array[11] eq "$del_network_errors[2]"
+)
                {
                        $del_network3_error++;
                }
                elsif ($cdr_post_array[11] eq "$del_network_errors[3]"
+)
                {
                        $del_network4_error++;
                }
                elsif ($cdr_post_array[11] eq "$del_network_errors[4]"
+)
                {
                        $del_network5_error++;
                }
                if ($cdr_post_array[11] eq "$del_system_errrors[0]")
                {
                        $del_system1_error++;
                }
                elsif ($cdr_post_array[11] eq "$del_system_errrors[1]"
+)
                {
                        $del_system2_error++;
                }
                elsif ($cdr_post_array[11] eq "$del_system_errrors[2]"
+)
                {
                        $del_system3_error++;
                }
                elsif ($cdr_post_array[11] eq "$del_system_errrors[3]"
+)
                {
                        $del_system4_error++;
                }
                elsif ($cdr_post_array[11] eq "$del_system_errrors[4]"
+)
                {
                        $del_system5_error++;
                }
#---------------------------------END OF ERROR SECTION----------------
+-------------------------------------------------------------#

    }
}
print "Completed postpaid section....\n";

[download]

Replies are listed 'Best First'.
Re: Making program readable
by Athanasius (Archbishop) on Oct 06, 2015 at 07:42 UTC

    Hello ravi45722,

    Here I cant use "sub" because in error sections the @network array, @subscriber array, @system errror is too big to pass to a function. It may slow down or even memory outage sometimes.

    In Perl, arguments to subroutines are passed by reference, not by value. Within the sub, the special variable @_ contains aliases to the passed-in arguments. It’s usual practice to begin by making local copies:

    my ($arg1, $arg2, ...) = @_;

    [download]

    but that isn’t required: you can always access the arguments directly as $_[0], $_[1], etc.

    OK, so there’s still some overhead involved in making the aliases (I haven’t benchmarked it). But you can bypass most of this overhead by passing an array reference instead of an array:

    sub some_function
{
    my ($network_ref, $subscriber_ref, $system_ref) = @_;

    for (@$network_ref)
    {
        ...
    }

    ...
}

...

some_function(\@network_error, \@subscriber_error, \@system_error);

    [download]

    See perlsub.

    Hope that helps,

    Athanasius <°(((>< contra mundum Iustus alius egestas vitae, eros Piratica,

[reply]
[d/l]
[select]
Re: Making program readable
by RichardK (Parson) on Oct 06, 2015 at 08:36 UTC

    I have no OO techniques to suggest, just write clearer code.

    Don't repeat yourself so much. This :-

            if ($intermediate_array[12] eq "GSM" && $intermediate_array[9]
+ eq "MSubmit")
        {
            $intermediate_MO_cnt++;
        }
        if ($intermediate_array[12] eq "GSM" && $intermediate_array[9]
+ eq "MSubmit" && $intermediate_array[7] eq "MsgAccepted")
        {
            $total_MO_success++;
        }

    [download]

    could be written as :-

        if ($intermediate_array[12] eq "GSM" && $intermediate_array[9] eq 
+"MSubmit") {

        $intermediate_MO_cnt++;
        if ( $intermediate_array[7] eq "MsgAccepted") {
            $total_MO_success++;
        }
    }

    [download]

    It's clearer and easier to see what your intent was, and there are fewer places for bugs to hide.

    Variable names like $del_user1_error , $del_user2_error, .. $del_userN_error are always a sign that you should have used any array and you're writing too much code. So instead you could do something like this :-

        my @del_user_error_count;

    for my $i (0..4) {
        if ($cdr_post_array[11] eq $del_user_errors[$i]) {
            $del_user_error_count[$i]++;
            last;
        }
    }

    [download]
[reply]
[d/l]
[select]

      I replaced the my if-else loops with these. 

          my @del_user_error_count;

    foreach my $i (0..4) {
        if ($cdr_post_array[11] eq $del_user_errors[$i]) {
            $del_user_error_count[$i]++;
            last;
        }
    }

      [download]

      Its decreased 42 lines but its increasing the execution time. I see this through benchmark.

      For if-else loop it gives

       the code took:52 wallclock secs

      After replacing the code with foreach

       the code took:67 wallclock secs
[reply]
[d/l]
[select]
        "...I see this through benchmark..."

        You might try Iterator::Simple:

        #!/usr/bin/env perl

use strict;
use warnings;
# use feature qw (say);
use Iterator::Simple qw(iter);

my $iterator = iter( [ 1 .. 4 ] );

while ( defined( my $i = $iterator->() ) ) {
    # warp action goes here
}

        [download]

        See also Bleeding edge as well as Re^2: Useful number of childs revisited.

        Regards, Karl

        «The Crux of the Biscuit is the Apostrophe»

[reply]
[d/l]

        Its decreased 42 lines but its increasing the execution time. I see this through benchmark.

        Ignore it, its not important :)

[reply]

      Ya... I understand what you said. Thank you. I will change it. Is there any more changes that can increase my program readability or to reduce the code???

[reply]

        Is there any more changes that can increase my program readability or to reduce the code???

        What does your program look like now ravi45722?

[reply]
Re: Making program readable
by graff (Chancellor) on Oct 07, 2015 at 03:45 UTC
    I'd be inclined to maximize the use of distinct subroutines for specific subsets of data, as well as using more hashes to organize things according to types and labels. Here's how I would try to structure the first part of the OP code. (I think the array idea mentioned above will help a lot with the rest.)

    I have no way of testing this proposal in a relevant way, but I think the layout is basically in line with what you're trying to do. (Note that I'm renaming your "$AO_ESMEdep_error" to a hash key spelled "AO_userdep_error", to maintain parallelism between the GSM and SMPP conditions - I hope that doesn't warp things too much for you (the parallelism is worth it, I think). 

    my %tallies;
my %errors = {
    userdep_error => \@subscriber_error,
    network_error => \@network_error,
    system_error => \@system_error,
};

sub do_GSM
{
    my $fields = shift;
 
    if ( $$fields[9] eq 'MSubmit' ) {
        $tallies{MO_count}++;
        if ( $$fields[7] eq 'MsgAccepted' ) {
            $tallies{MO_success]++;
        }
    }
    if ( $$fields[13] eq 'SMPP' ) {
        $tallies{AT}++;
    }

    for my $type ( keys %errors ) {
        if ( any { /^\Q$$fields[11]\E$/ } @{$errors{$type}} ) {
            $tallies{"MO_$type"}++;
        }
    }
}

sub do_SMPP
{
    my $fields = shift;

    if ( $$fields[9] eq 'ESubmit' ) {
        $$tallies{AO_count}++;
        if ( $$fields[7] eq 'MsgAccepted' ) {
            $tallies{AO_success}++;
        }
    }
    if ( $$fields[11] =~ /SMSC/ ) {
        for my $type ( keys %errors ) {
            if ( any { /^\Q$$fields[11]\E$/ } @{$errors{$type}} ) {
                $tallies{"AO_$type"}++;
            }
        }
    }
}

my %subs = ( GSM => \&do_GSM, SMPP => \&do_SMPP );

foreach my $Intermediate_file (@Intermediate_files)
{
    unless ( open (INTER, $Intermediate_file )) {
        warn "open failed for $Intermediate_file\n";
        next;
    }
    while (<INTER>)
    {
        chomp;
        my @fields = split(/\|/);
        &{$subs{$fields[12]}}( \@fields );
    }
}
print "Completed the Imtermediate section....\n";
...

    [download]
    I also altered the handling of open failures and shortened the variable names.
[reply]
[d/l]

      Thank you very much for trying to simplify my code. But I think you aware of GSM & SMPP differences. In GSM (Person to person) there is user_errors. In SMPP (Application to person) there is ESME_errors. We cannot club them. It's totally different. Thanks once again for reply.

[reply]
A reply falls below the community's threshold of quality. You may see it by logging in.

Back to Seekers of Perl Wisdom