comment on

Why is it so horrible to read??

Because the haphazard indentation makes it almost impossible to tell, at a glance, where each logical block of code ends. Here’s the original version of sub write_register, with a single comment added:

sub write_register {
my ($date,$client_ip,$client_imsi) = @_;

 $output=qx(snmpwalk -v2c -t1 -c $community $client_ip $snmp_fver 2>&1
+);
 chomp($output);
        if( $output eq "Timeout: No Response from $client_ip" ) {
        return; 
        }
        else{
                   my @result=split(/:/,$output);
                   if ($result[3]){
           $fver=$result[3];
                   $fver=~s/ //g;
                   $fver=~s/\n//g;

                   }else{
            exit;
            }
               }    # X

        if($bsid){
                system "sed -i '/$client_imsi/d' $path/leases_list.txt
+";
        system 'echo "'.$date.','.$client_ip.','.$client_imsi.','. $fv
+er.'" >> '.$path.'/leases_list.txt';
        }
        
}
[download]

Now, quickly: which block of code is terminated by the right brace named X?

Here’s a preliminary clean-up of the code, with consistent indentation:

sub write_register
{
    my ($date, $client_ip, $client_imsi) = @_;

    $output = qx(snmpwalk -v2c -t1 -c $community $client_ip $snmp_fver
+ 2>&1);
    chomp $output;

    if ($output eq "Timeout: No Response from $client_ip")
    {
        return; 
    }
    else
    {
        my @result = split /:/, $output;

        if ($result[3])
        {
            $fver =  $result[3];
            $fver =~ s/ //g;
            $fver =~ s/\n//g;
        }
        else
        {
            exit;
        }
    }   # X

    if ($bsid)
    {
        system "sed -i '/$client_imsi/d' $path/leases_list.txt";
        system 'echo "' . $date . ',' . $client_ip . ',' . $client_ims
+i . ',' .
                          $fver . '" >> ' . $path . '/leases_list.txt'
+;
    }
}
[download]

Easier to follow now? Yes, and that makes it easier to spot where it can be improved:

As noted by Anonymous Monk++, the variables $output and $fver are never declared. In addition, the variable $bsid is neither declared nor initialized anywhere in the script.
Perl allows you to return, exit, or die at any point in the code. Since these statements are forms of flow control, the code can often be simplified when these statements are used. For example, this:
```
if (condition)
{
    return;
}
else
{
    ...
}
[download]
```
is logically equivalent to the shorter and simpler:
```
return if condition;
...
[download]
```
It’s bad practice to exit from a subroutine. You should die (i.e., throw an exception) instead: that way, the subroutine’s clients have the option to either handle the exception locally or allow it to propagate upwards and so terminate the script.

Here’s an improved version of the subroutine:

sub write_register
{
    my ($date, $client_ip, $client_imsi, $bsid) = @_;
    chomp(my $output = qx(snmpwalk -v2c -t1 -c $community $client_ip $
+snmp_fver 2>&1));
    return if $output eq "Timeout: No Response from $client_ip";
    my @result = split /:/, $output;
    die unless $result[3];
    my $fver = $result[3];
       $fver =~ s/[ \n]//g;

    if ($bsid)
    {
        system "sed -i '/$client_imsi/d' $path/leases_list.txt";
        system 'echo "' . $date . ',' . $client_ip . ',' . $client_ims
+i . ',' .
                          $fver . '" >> ' . $path . '/leases_list.txt'
+;
    }
}
[download]

As a rule, the shorter and simpler the code, the easier it is to debug and maintain. Disclaimer: I haven’t looked at the code’s logic, only its form.

Hope that helps,

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

In reply to Re^3: Am I forking properly ? by Athanasius
in thread Im I forking properly ? by leostereo

Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!

Titles consisting of a single word are discouraged, and in most cases are disallowed outright.

Read Where should I post X? if you're not absolutely sure you're posting in the right place.

Please read these before you post! —

Posts may use any of the Perl Monks Approved HTML tags:

a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, details, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, summary, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr

You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)

	For:		Use:
	&		`&`
	<		`<`
	>		`>`
	[		`[`
	]		`]`

Link using PerlMonks shortcuts! What shortcuts can I use for linking?

See Writeup Formatting Tips and other pages linked from there for more info.


Syntactic Confectionery Delight
	PerlMonks