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":



  • 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:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.