Beefy Boxes and Bandwidth Generously Provided by pair Networks
Do you know where your variables are?

comment on

( [id://3333]=superdoc: print w/replies, xml ) Need Help??

Hello leostereo,

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'; } }

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' +; } }

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

  1. 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.
  2. 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 { ... }
    is logically equivalent to the shorter and simpler:
    return if condition; ...
  3. 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' +; } }

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

Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post; it's "PerlMonks-approved HTML":

  • Are you posting in the right place? Check out Where do I post X? to know for sure.
  • Posts may use any of the Perl Monks Approved HTML tags. Currently these include the following:
    <code> <a> <b> <big> <blockquote> <br /> <dd> <dl> <dt> <em> <font> <h1> <h2> <h3> <h4> <h5> <h6> <hr /> <i> <li> <nbsp> <ol> <p> <small> <strike> <strong> <sub> <sup> <table> <td> <th> <tr> <tt> <u> <ul>
  • Snippets of code should be wrapped in <code> tags not <pre> tags. In fact, <pre> tags should generally be avoided. If they must be used, extreme care should be taken to ensure that their contents do not have long lines (<70 chars), in order to prevent horizontal scrolling (and possible janitor intervention).
  • Want more info? How to link or How to display code and escape characters are good places to start.
Log In?

What's my password?
Create A New User
Domain Nodelet?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others chilling in the Monastery: (3)
As of 2024-05-25 05:09 GMT
Find Nodes?
    Voting Booth?

    No recent polls found