Beefy Boxes and Bandwidth Generously Provided by pair Networks
The stupid question is the question not asked
 
PerlMonks  

comment on

( [id://3333]=superdoc: print w/replies, xml ) Need Help??
OK, I'm not looking at the macro scale, just the micro scale. Here are a few random observations.
use LWP::Simple; use LWP::UserAgent; use HTTP::Request; use HTTP::Request::Common qw(GET); use HTTP::Response;

You never use HTTP::Request::Common::GET so why do you import it? LWP::UserAgent will load HTTP::Request and HTTP::Response for you. As far as I can see you never actually use LWP::Simple.


$|=1; #Disable buffering to allow instant output of text.

Is there a paricular reason you do this? If not, don't.


our $editorPassword = "**********"; our $language = "en"; # language code for wikipedia namespace our $enabled = "true"; # set to false to take offline/disable

Is there a paricular reason you use our? If not use my.


if ($enabled eq "true") {

It is my number one rule in programming: "use the most natural representation of things unless there is a reason to do otherwise".

If you want $enabled to be a boolean then use Perl's natural concept of "true" or "false" and simply say:

if ($enabled) {

&getInput;

Do not use the special &-syntax to call subroutines unless you understand and require its special semantics.

getInput();

my @gettheip = split(/\./,$ENV{'REMOTE_ADDR'}); my $remoteHost = "$gettheip[0].$gettheip[1].$gettheip[2].$gettheip[3]" +;

Why do you split and rejoin the IP address?


sub readEditTokens { &checkFileCanBeAccessed($editTokenFile, "READ"); open(TOKENFILE,"$editTokenFile") || &error("Cannot open edit token + file."); flock(TOKENFILE, 2) || &error("Cannot lock edit token file.") +; my $editTokens = <TOKENFILE>; flock(TOKENFILE, 8); close (TOKENFILE); return ($editTokens); }

The checkFileCanBeAccessed is ill-concieved. It adds complexity, introduces a race condition and delivers no beniefit.

You have useless quotes in "$editTokenFile".

You use the special legacy 2-arg open() syntax in which perl parses the second argument into two parts: mode and filename. It is simpler and clearer to use the 3-arg format.

You are using the global file handle TOKENFILE. This habit could cause you troble in future. You could local(*TOKENFILE) to make your changes to the global handle temporary but better to simply not use a global handle.

You don't bother to check for errors on close(). This is not too bad but if you localised the filehandle you wouldn't need to close it explicitly (unless to check for errors).

Your indentation is confusing.

It is more conventinal to use or not || for flow control.

Error messages should contain the error.

You are taking an exclusive lock when you are just reading.

There's no reason to unlock a handle you are about to close anyhow.

sub readEditTokens { open(my $TOKENFILE, '<', $editTokenFile) or error("Cannot open edit token file: $!"); flock($TOKENFILE, 1) or error("Cannot lock edit token file: $!"); my $editTokens = <$TOKENFILE>; return ($editTokens); }

However, consider instead File::Slurp.


print STDOUT "Content-type: text/html\n\n";

Why are you making the filehandle explict?


In reply to Re: Could I get some feedback on my code please? by nobull
in thread Could I get some feedback on my code please? by PockMonk

Title:
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?
Username:
Password:

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

How do I use this?Last hourOther CB clients
Other Users?
Others learning in the Monastery: (4)
As of 2024-04-25 16:59 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found