(minor typo - it's "aesthetic".) Check out perldoc perlstyle - that's a great place to start.

I bookmarked a Slashdot comment (!) a few months ago that I thought raised some points that are mostly worthy of consideration.

Along similar lines, the much-missed Abigail's coding guidelines contain style recommendations that demand reading.

Update: put <em> tags around the word `mostly'.

I bookmarked a Slashdot comment (!) a few months ago that I thought raised some points that are mostly worthy of consideration.

Worthy of consideration, certainly, and many of them I quite agree with. I do think, however, that some of them are a bit misguided. I'm not convinced that $Mixed_Case_Globals, for example, are a great idea. Here are some more of my reservations...

His point 6, about blocks, I tend to disagree with; to me, that's redundant and wastes vertical space, resulting in the need to scroll more than is really necessary in order to see all of a function or block; this can really impede the reader's ability to follow the code in some cases, because when a block goes past a screenful it's *harder* to see where it starts and ends, not easier. If you pull up the braces inside that block onto their respective previous lines and let the indentation speak for itself, you can see more (if not the whole thing) at once. The indentation makes it visually clear where blocks end even if your text editor _doesn't_ do brace matching (which it does if it's not fundamentally lame). But if you put every brace on its own line, you can almost double the length of the function, in terms of the number of lines.

Point 11, about map, depends greatly on the background of your programmers. If you expect the code to be maintained by someone coming from a background in languages such as Java, it's probably good advice, but if the person doing code maintenance can be expected to have a couple of years' background in Perl or lisp, this concern goes away entirely. In some cases map adds significant clarity (assuming you understand lists).

I absolutely disagree about unless. It may not be common in other computer languages, but it's a very straightforward cognate for the English word "unless" and means exactly what you would think it would mean.

Using an explicit main() is completely pointless unless your main goal is to confuse C programmers about what language they're reading. (OTOH, he suggests putting an explicit exit before your post-defined subroutines, which is good practice IMO.)

Regarding one-letter variables: the point is valid, but his stance is IMO a bit hard-nosed. I like to use a one-letter variable when a given script or function really has one main object that it keeps working with a lot -- such as the record object that a function in a DBI script is working with, or the one WWW::Mechanize object that a web automation script is using every line or two. However, it is certainly true that overuse of short variable names is to be reserved for the Obfuscated Code section.

Point 18, about minimizing implicit pronouns, is one of the worst, most extremely misguided pieces of Perl style advice EVER. Cluttering the program with a couple of hundred stupid unnecessary extra variables that only hang around for a couple of lines does NOT make it more clear.

I agree with 19 in principle, theoretically, but I'm guilty of violating it without remorse with alarming regularity, usually when the scalar is either an index into the array or the iterator in a foreach over the keys of the hash in question.

Number 22 I also tend to disagree with: in many cases all it does is add extra lines, pushing more of the function off to the next screenful.

Number 28 reminds me of COBOL. Either write POD or don't, but this style suggestion is silly either way.

Number 29 seems to have forgotten that we have __END__ for this purpose.

OTOH, suggestion 14 (about having a routine that logs debug messages) is something I consider to be Very Good Style that is often missed, and many of the other suggestions are quite good too. It's definitely worth looking at and considering -- just don't take it as some kind of definitive authority; many of the points it makes seem geared more toward verbosity and C-like-ness than clarity.

---

"In adjectives, with the addition of inflectional endings, a changeable long vowel (Qamets or Tsere) in an open, propretonic syllable will reduce to Vocal Shewa. This type of change occurs when the open, pretonic syllable of the masculine singular adjective becomes propretonic with the addition of inflectional endings."  — Pratico & Van Pelt, BBHG, p68

Having maintained other peoples code for far too many years, the one thing I would say is comment. Lots and lots of comment. Not just:

#this bit does xyz
sub xyz....
[download]

which can be difficult to see in a long script, but

#######################################
# subroutine xyz - does xyz, abc, zz plural z alpha
#######################################
sub xyz..
[download]

Or your own personal variant of it. Also, give variables reasonably explanatory names, and make fairly liberal use of whitespace.

Those would be my suggestions anyway. Why not post a code snippet and let the monks bitch about it? :-)

charles.

Addendum: applause for wanting to make your code easier to read though - most people wouldn't bother!

For instance, I think perlstyle says you should omit the last ; in a block.

No, that's not what it says at all. It says:

Semicolon omitted in "short" one-line BLOCK.

where "one-line" should be taken to include the curlies, and "short" should be taken to mean something that can be seen as a kind of visual "pill". So it's saying it's okay to omit the semicolon in

    sort { $a <=> $b } @foo;
[download]

but not in

    sort {
         $a <=> $b;
    } @foo;
[download]

So for the most part you're basically in violent agreement with perlstyle.

Apologies, that'll teach me to do my research properly! :-)

heres a snippet of code which Ive cleaed up a bit - Ive posted earlier viersions of this elsewehere on perlmonks and added some of the suggestions that I received.



#!/usr/bin/perl -T
# mship.cgi
# Just add hosts ip addresses, fqdn, or just hostnames
# gives a simple status of hosts up or hosts down
# next step is to add logging. The aim is not
# a full featured network monitor, just a quickie list
# of whats up and whats down - Its probably not useful 
# in a large environment...
# Ted Fiedler <fiedlert@gmail.com>
use strict;
use warnings;
use Net::Ping;
use CGI qw/:standard/;
use POSIX qw(strftime);

# Clean up our UNIX environment
# for more infor read perldoc perlsec
$ENV{'PATH'} = '/bin:/usr/bin';
delete @ENV{qw(IFS CDPATH ENV BASH_ENV)};

my (@hosts, @uphosts, @downhosts);

if ( -e "ship.cfg" ) {
    open (CFGFILE, "ship.cfg") || die $!;
    while (my @TMP=<CFGFILE>) {
        next if ( /#/ );
        # Lets untaint the data after making sure it only matches
        # word characters... For more info read perldoc perlsec.
        foreach $_(@TMP) {
            if ($_ =~ /^([\w.]+)$/) {
                $_ = $1; 
                push (@hosts, $1);
            } else {
                next;
            }
        }
    }
} else {
    # or enter your hosts here...
    # This is the most secure/preferred way to do this
    @hosts = qw/sapdev0 sapprd0 saptst0 pcbackup vader/;
}

# I haven't decided what else to do here - it just semantics...
my @verbage=("are", "hosts", "are", "hosts");

# udp is default
# other values can be icmp (if youre root)
# or TCP, which is expensive on the wire
my $p = Net::Ping->new('udp');

# Iterate through my hosts and get their status
foreach my $host(@hosts) {
    chomp($host); # Cleans up ship.cfg if you're using it
    if ($p->ping($host)) {
        push (@uphosts, $host); # its either UP
    } else {
        push (@downhosts, $host); # or its DOWN...
    }
}

$p->close();

print header, start_html;

print "<tt>\n";
print "\n<center><h3>MSHIP - my status of host IP's</h3></center>\n";
my $now=strftime "%m/%d/%Y %H:%M:%S", (localtime);
print "<center>$now</center>\n";
print hr;

# Just cleaning up the verbage is are host hosts etc...
if (scalar(@downhosts) == 1 ) {
    $verbage[2]="is";
    $verbage[3]="host"
} 
if (scalar(@uphosts) == 1 ) {
    $verbage[0]="is";
    $verbage[1]="host";
}

# We don't care about things that don't exist...
unless (scalar(@downhosts) == 0) { print p,"There $verbage[2] ",scalar
+(@downhosts),"  $verbage[3] unreachable\n",br }
print "There $verbage[0] ",scalar(@uphosts)," $verbage[1] alive\n";

print p,"The following $verbage[1] $verbage[0] alive: \n",br;
foreach my $uitem (sort @uphosts) { # sort is for cleanliness...
    print li("$uitem\n"),br;
}
# Again, we don't care about things that don't exist
unless (scalar(@downhosts) == 0) { print p,"The following $verbage[3] 
+$verbage[2] unreachable: \n",br }
foreach my $ditem (sort @downhosts) {
    print li("$ditem\n"),br;
}
print "</tt>\n";

print start_form,
    p,submit('Refresh'),
    end_html;
[download]

Regarding this:

    open (CFGFILE, "ship.cfg") || die $!;
    while (my @TMP=<CFGFILE>) {
        next if ( /#/ );
        # Lets untaint the data after making sure it only matches
        # word characters... For more info read perldoc perlsec.
        foreach $_(@TMP) {
            if ($_ =~ /^([\w.]+)$/) {
                $_ = $1; 
                push (@hosts, $1);
            } else {
                next;
            }
        }
    }
[download]

That's totally wrong. Ordinarily, you either read from a file a line at a time, using while, or you read the entire (rest of the) file by assigning to a list. You can't meaningfully combine the two approaches, which is what it looks like you've tried to do. Below is how to write your loop using the one-fell-swoop approach, with the other transformations included:

    open CFGFILE, "< ship.cfg" or die "read ship.cfg: $!";
    @hosts =
        map { /^([\w.]+)$/ ? $1 : () }
        grep !/#/,
        <CFGFILE>;
    }
    close CFGFILE;
[download]

Here's something similar but using a line-at-a-time approach:

    open CFGFILE, "< ship.cfg" or die "read ship.cfg: $!";
    while (<CFGFILE>) {
        /#/ and next;
        /^([\w.]+)$/ or next;
        push @hosts, $1;
    }
    close CFGFILE;
[download]

hth.

Looks fine to me, but I am not exactly an expert myself. :-) One thing that I guess could be written differently is this:


foreach $_(@TMP) {
            if ($_ =~ /^([\w.]+)$/) {
                $_ = $1; 
                push (@hosts, $1);
            } else {
                next;
            }
        }
[download]


Something like this, perhaps:



foreach (@tmp) {
  push @hosts, $1 if /^([\w.]+)$/;
}
[download]

I may be missing something blindingly obvious, though. :-)

Looks good to me too (but that's not saying that much!)

If a print statement is a bit unwieldy I often use join to get it to read better.

print join "\n",
  '<center>',
  '<h3>MSHIP - my status of host IP's</h3>',
  '</center>';
[download]

I would have considered $ping instead of $p.
I prefer a space either side of an = (sometimes you do too, sometimes you don't!).
I think that 2 space tabs use less real estate than 4 if there is deep nesting.

foreach $_(@TMP)
could be written as
for (@TMP)

} else {
Is that what they call cuddled?
It's controversial but I put the else on a new line.

All very minor.

I can't speak for anyone else, but I can't see much to complain about in that code. I try not to use single line {} blocks unless its very short is the only thing that springs to mind.

Anyone else?

foreach my $uitem (sort @uphosts) { # sort is for cleanliness...
    print li("$uitem\n"),br;
}
[download]

I prefer to use map over foreach when possible:

print join br, map li $_, sort @uphosts;
[download]

But maybe that's just because I like writing LISP in Perl :)

"There is no shame in being self-taught, only in not trying to learn in the first place." -- Atrus, Myst: The Book of D'ni.

print "<tt>­\n";
print "\n<c­enter­><h3>­MSHIP - my statu­s of host IP's<­/h3><­/cent­e
+r>\n­";
my $now=­strft­ime "%m/%­d/%Y %H:%M­:%S", (loca­ltime­);
print "<cen­ter>$­now</­cente­r>\n"­;
print hr;
[download]

Personally, I'd probably use a heredoc for this:

my $now=strftime "%m/%d/%Y %H:%M:%S", localtime;
print <<"END";
<tt><center><h3>­MSHIP - my statu­s of host IPs</h3></center>
<center>$now</center>
<hr />
END
[download]

Of course, I wouldn't write HTML like that, but continuing in that direction will only send me way off-topic...

=cut
--Brent Dax
There is no sig.

For subroutine comments, use POD, not ugly flowerbox comments. Better still, name your subroutines and clearly label the input params, and your documentation will take care of itself.

"There is no shame in being self-taught, only in not trying to learn in the first place." -- Atrus, Myst: The Book of D'ni.

For subroutine comments, use POD,

I disagree with that. POD is used to make user documentation. Unless you play silly tricks with =for or =begin, anything that you document with POD, will end up in the user manual. That gives you two options:
1. You will make some subroutines part of the API while they shouldn't be. This breaks encapsulation, and in the future, it will give you the choice to break backwards compatibility, or to support code you'd rather not support. It will restrict your freedom to redo the internals of your module.
2. Document that the description that follows is about code the user shouldn't use. This makes the documentation clumsy, as it could easily be littered with pieces of documentation the user isn't supposed to use.

POD is for the user, comments are for the coder. And while they may be the same, they usually aren't.

Lots and lots of comment.

When I was a programming novice, I didn't comment much.
When I was a programming acolyte, I commented a lot.
Now I am a programming master, and I try to minize the comments in my programs.

Why, you may ask. Doesn't more comments make it easier to understand what code does? Yes, it does - however, comments don't solve the cause. Comments fix the symptoms. Comments are used when the program isn't clear. If you write clearer programs, you need less comments.

It's like taking painkillers. They cause the headache to go away, but it's better to stop banging your head.

One of the best things you can do is look at the perlstyle pod if you have a Unix or Unix like system.

I wish I could help more but I'm still tring to understand all this myself.

rlb3

2006-09-14 Retitled by planetscape, as per Monastery guidelines

Yes, I have been told about my informal writing style by others. I have purchased a copy of "Elements of Style", but have only read the intro thus far. My writing style and my coding style are similar, as in I never really had to write for other people. Now I have to write for other people and I am seeing how it can be a problem for others. Thanks for your comments.

• As others have mentioned, spelling and grammer are very important. Learn to express yourself in English, first. Then move on to programming.
• Well written code should read very much like English prose. Think of each statement as a sentence. Variable names are like nouns. Function calls and operators are like verbs. Functions and code block are like paragraphs. If you choose the right "words" and "sentences", your code will be easy to read.

  If you can't understand a statement without pausing to think, re-write it. If it isn't obvious why the statement is in the current function, document the function better. Explain how the statement fulfils the purpose of the function. If the statement doesn't have anything to do with the function's purpose, move it elsewhere or delete it.

• Use function calls. If you can make your program more self-descriptive by wrapping a block of code with a meaningful name, do it. Even if the function is only a single line of code, if it's complicated enough, it belong in it's own function. Remember, function calls are fast. Computer time is much cheaper than programmer time. You can optimize parts of your program later if it's slow: but you can't optimize what you can't understand.

  Not every line of code belongs in a function, of course. Optimize to minimize confusion: if a descriptive name is more clear than reading the corresponding code, the code belongs in a function of it's own.

  Document your functions. Explain their purpose, their limitations, and any assumptions about how they will be used or called. Explain any side effects. Document the error handling (and think about error handling).

• Avoid vague variable names, such as "verbiage". Every aspect of your program should contribute to the documentation of your program.

  Even if you're throwing away a big section of "junk data", make what you're doing and why as clear as possible. The variable names should explain what the source of the data was. The search or filter function call should explain what data you want to keep. The pod documentation for the calling function should explain the requirements for the function. Based upon those requirements, it should be clear why you need to keep the data : if it isn't, document it.

• Write for code maintenance. Keep expressions short and simple. It's easier to debug expressions that have been broken down into sub-expressions. It's also often easier to prove them correct. And English style guides say that short sentences are easier to understand than long ones. That's true in Perl, too.
• Use pod for documentation. In my code, I keep the user documentation (how to run the program) distinct from code's documenation. I still use pod for both.

  I use normal pod for the user documentation. That way, "perldoc <programname>" always explains how to run the program.

  For the code documentation, I use a special feature of pod. I mark my code documentation with the '=begin <translator>' and '=end <translator>' pod directives. (I call my translator program "programerdocs".) My programmerdocs pod translator program just translates 'programmerdocs' lines into normal pod, by adding an '=pod', then filters them through the normal pod translator. This way, I can maintain two independent sets of pod documentation: one for end users, and one for coders and testers.

  My goal is that a skilled tester could take the internals documentation, and write unit/integration tests for all the functions, without ever reading a line of code. If they can do that, I know it's properly documented.

• In general, avoid the use of constants. If you have to use them (such as hardcoding a list for security), put them at the front of the function so they can be found and updated quickly by a code maintainer.
• Not many people seem to do this, but I like to comment the terminating brace of function calls, if statements, loop constructs, and so forth. I generally try to take the opportunity to sum up the purpose of the construct, restating what the purpose of the matching section was. Again, I'm trying to avoid work for the mainance coder.

  If you need to add a statment before the end of a loop, would you rather see:

       }
      }
  }
  }
  [download]

  or

          } # end loop over records from input file
     } # end loop over files
  } # end read_records()
  } # end block of variables private to read_records()
  [download]

  It also helps me out when I have a program with one too many or too few braces: I can quickly match what a brace actually does match with (using % in vi), and compare against what it should match.

• Code so that the most junior programmer understands. Sometimes it helps to wander over to the desk of someone who knows less Perl than you do. Ask him/her which of two alternative Perl expressions are simpler to their eyes. Repeat with all the junior coders you can find. If there's a majority opinion, respect it. If a novice can understand your code, an expert can, too.
• However, write for your intended audience. If everyone at work is a Perl guru, and there is a corporate standard that requires a given skill level for Perl hires, then you can assume more knowledge about Perl in your documentation.

  Never assume your audience knows your project's business rules, implicit assumptions, or so forth unless there is an excellent reason to assume this will be true. Remember that programming staff come and go, so try to ensure that the costs of excessive documentation are balanced against the risks of information loss.

To sum up: make every aspect of your code a form of documentation. Don't use $_ when you could use a more meaningful name. Try coding defensively: will your current line of code still make sense after someone else throws 100 lines badly formatted code in the middle? If so, then the code is probably self-documenting enough.

The most useful book on programming that I ever read was called "Debugging C": it taught that the most effective way to debug programs was to write them in a simple, straightforward way in the first place. It's very good advice.

Good luck! Keep practicing! The first step towards better style is trying to learn!

--
Ytrew Q. Uiop

The fat cat sat on the mat. The fat cat was tired. The fat cat had just eaten. The fat cat ate a lot. That made the fat cat sleepy. The fat cat went to sleep on the mat.

The fat cat sat on the mat. The fat cat was tired. The fat cat had just eaten. The fat cat ate a lot. That made the fat cat sleepy. The fat cat went to sleep on the mat.

It is far better to write the simplistic text above than the purportedly "more adult" text below, in my opinion.

"The aforementioned creature, grown senecent in the ways of corpulence, had taken up residence upon an entanglement of fibrous matter. Bestirring itself to the siren call of the Sandman's lair, it gravitated in a state of satiety, brought on by an act of pedestrian consumption, noteable only by it's excess. Led thereby to the banquet of Morpheus, it took up the preoffered cup, paused, and drank deeply."

The second paragraph is similar to a lot of code I see in the world today: it suffers from trying too hard. It has several faults. It's excessively wordy. It uses too many confusing metaphors. For example, "Morpheus's banquet" refers to sleeping, not eating.(Morpheus was the ancient greek God of Dreams.)

It contains both grammar mistakes ("it's" rather than "its") and spelling errors ("preoffered" is particularly confusing, as it looks like "pre-offered" rather than "proffered").

It also fails to mention that the creature in question is really a cat. Given that the story is about the cat, it's a major flaw.

Suppose I hire a 14 year old kid as a junior editor. If I tell him "change the cat from fat to thin", and "make him drink, not eat", I know he can fix the first story with ease. I also know that he'll have a harder time with the second story: it will cost me more money, because he'll get confused, and take longer trying to figure it out. It would take a lot of editing to fix the second story.

If I were to edit the original text, I would find that there's not as much wrong with it. The word "fat" is repeated needlessly. The paragraph doesn't stand alone: it assumes that a specific cat has been previously identified, by the use of the word "the" rather than "a" when refering to the cat. The notions of sleep and causation are repeated.

I would probably write the story as:

"Once upon a time, there was a fat cat. The cat sat on a mat. It was sleepy, because it had just eaten a lot. So, it went to sleep on the mat."

Note that it's still readable by a grade school student, or by a non-native English speaker. I've taken seven years of French in school, and I still stumble when trying to read children's books. I imagine non-native programmers feel similarly with English.

Remember, the purpose of programming is not creative expression, but rather clear exposition. The CPU needs to know what to do: the maintainer needs to know why you did it. No one needs to know that you passed grade 13 English with a 97% average (except maybe your university English professor).
--
Ytrew

Explain more code to your co-workers.

I believe I was told it is a "Retronym" ;). It was named Pearl because Larry thought it sounded neat, changed it to Perl shortly after finding an existing language named Pearl. Then, later he came up with Practical Extraction and Report Language (interestingly, the "and" works with the orignal name :). I believe that was the stroy I was told :).

Yes, that's essentially what happened--except for the unfortunate fact that that's not what "retronym" means. Retronyms are things like "black and white TV", and "manual transmission", where the modifier merely tells you what "TV" and "transmission" used to mean all the time. Once you start looking for retronyms you find them all over the place: "dial telephone", "snail mail", "area rug", and pretty much "analog <anything>" these days. They're a lot of fun to think about.

Unfortunately, there's no decent word that I know of for an acronym definition made up after the acronym. I believe the OED says "later rationalizations" about Perl's glosses. I think "fakeronym" is cute, but it seems like we should really make up an acronym for it. We could call it a LATE, but I don't know what that stands for yet... :-)

• the else is easier to see if it is not cuddled:

  rather than this:
  
  } else {
  
  use this:
  
  }
  else {
  [download]

  which then gives the braces the same relative position fo the if and the else.
• Carefully consider NOT using the unless it can be quite counter-intutitive when reading code.
Code is easier to read and flows more logically (IMHO) if you use block conditionals, for example:

This:

unless ( scalar(@downhosts) == 0 ) {
    print p, "There $verbage[2] ", scalar(@downhosts), "  $verbage[3] 
+unreachable\n", br;
    }

rather than:

unless ( scalar(@downhosts) == 0 ) { print p, "There $verbage[2] ", sc
+alar(@downhosts), "  $verbage[3] unreachable\n", br }
[download]

I see others have pointed out some different approaches to dealing with filehandles and the like. Only other suggestion I would make is to use CGI more effectively by not printing HTML directly - always use CGI/

And finally, use perltidy. I have it in the toolbox in my Komodo so I can run it over my code any time. It actually mkes it easier to spot problems when I can see thngs like the indentation level go wrong! It also gives me a consistency of style I am too lazy to get any other way.

jdtoronto