Beefy Boxes and Bandwidth Generously Provided by pair Networks
"be consistent"
 
PerlMonks  

Code optomization

by sock (Monk)
on Jul 07, 2004 at 22:57 UTC ( [id://372594]=perlquestion: print w/replies, xml ) Need Help??

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

I've written a small script that retrieves several pages from different websites that list anonymous http proxies, then it sifts through the listed proxies and creates a formated output list off of those. This is one of my first real scripts and I know it could be optimized in many ways. I am not asking anyone to do it but to show me what I miss.
Guns don't kill people, ninjas do.

Replies are listed 'Best First'.
Re: Code optomization
by sgifford (Prior) on Jul 08, 2004 at 07:13 UTC
    Here are my comments:
    • You probably use strict and use warnings. That catches many kinds of common errors. To do this, you'll have to fix various things like declaring your variables.
    • When you're initializing the %info hash, consider using the "arrow" syntax. It's easier to read:
      our %info = (version => "0.3" format => 0, ... elite => 1, );
    • With your debug statements, IMHO they're clearer if you use this syntax:
      print "This is a debug message\n" if ($debug);
      Larry Wall's rule for how to arrange things is put the important stuff first, and in this case the message can server as documentation as well as a debugging string, so should probably go first.
    • What does this mean?
      return and "Failed to login\n" unless ($http_res->is_success);
    • Several of your subs have the exact same code at the end. If possible, you should find a way to factor that out.
    • Using the $& variable can slow down your script considerably, according to perlvar(1). I'm not sure if having it in an if ($debug) block helps or not. If so, maybe an eval would stop the performance hit if you aren't debugging.
    • The looping construct at the end is awkward. Something like this would be clearer:
      foreach my $d (@data) { if ($d->[5] == 1) { ... } }
    • Using constants for your array subscripts would make your script clearer, and if you use the use constant technique, won't slow things down at all.

    Of course, many of these are a matter of style; those are just my first impressions.

    Overall, looks like a nicely written script. Thanks for posting it!

      Ok, I've made some huge adjustments and code reduction. I've also applied use strict and use warnings. I'm getting one message in particular regarding the following:
      for ($3) { /transparent/ && $info{clear} == 1 && do {$use_this = 1; last;}; /anonymous/ && $info{anonymous} == 1 && do {$use_this = 1; last;}; /highanonymity/ && $info{spranonymous} == 1 && do {$use_this = 1; + last;}; }
      The actual warning:
      Use of uninitialized value in pattern match (m//) at ./ProxyHunter.pl +line 93.
      Using common sence i'm assuming that its raising an issue whenever only one of the 3 are matched. Would it be easier to use if/then statements?
      Guns don't kill people, ninjas do.
        As you say, that probably happens when $3 isn't defined. Adding something like:
        defined || last;
        at the top of that for block would probably do the trick.
      Updated version for your viewing pleasure. You were right about having redundancy so I compacted the 3 diff subs into one sub that just loops through an array of the target proxylisters. It also looks much cleaner. Applied use strict and use warnings but still get an odd warning, see other post. I've also changed the arrays used, thanks for that one too. See readmore for actual code. -----
      Guns don't kill people, ninjas do.

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: perlquestion [id://372594]
Approved by NetWallah
Front-paged by broquaint
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others contemplating the Monastery: (2)
As of 2024-04-24 23:04 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found