tcf03 has asked for the wisdom of the Perl Monks concerning the following question:
Ive been using PERL for 7 years plus, Ive been told on more than one occasion that I write ugly code - Can anyone point me towards any docs that emphasize asthetically pleasing and efficient code? I always seem to be able to use PERL to come up with solutions, I just want to be proud of my code... Recently Ive had to explain some code to a fellow co-worker, and was rather embarrassed by my lack of style.
Thanks
UPDATE - thanks all of these suggestions have been extremely useful 2006-09-14 Retitled by planetscape, as per Monastery guidelines ( keep:0 edit:12reap:0 )
Original title: 'aesthetics'
Re: improving the aesthetics of perl code
by matthewb (Curate) on Jan 21, 2005 at 14:40 UTC
|
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'.
| [reply] |
|
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
| [reply] |
Re: improving the aesthetics of perl code
by Tanktalus (Canon) on Jan 21, 2005 at 14:17 UTC
|
| [reply] |
Re: improving the aesthetics of perl code
by g0n (Priest) on Jan 21, 2005 at 14:26 UTC
|
Coding style is very much a matter of personal preference. For instance, I think perlstyle says you should omit the last ; in a block. Personally I disagree - if you add some code after that line later, it generates a syntax error unless you spot it and change it. But that might just be me.....
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....
which can be difficult to see in a long script, but
#######################################
# subroutine xyz - does xyz, abc, zz plural z alpha
#######################################
sub xyz..
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!
| [reply] [d/l] [select] |
|
sort { $a <=> $b } @foo;
but not in
sort {
$a <=> $b;
} @foo;
So for the most part you're basically in violent agreement with perlstyle. | [reply] [d/l] [select] |
|
Apologies, that'll teach me to do my research properly! :-)
| [reply] |
|
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;
| [reply] [d/l] |
|
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;
}
}
}
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;
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;
hth.
| [reply] [d/l] [select] |
|
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;
}
}
Something like this, perhaps:
foreach (@tmp) {
push @hosts, $1 if /^([\w.]+)$/;
}
I may be missing something blindingly obvious, though. :-) | [reply] [d/l] [select] |
|
|
|
|
print join "\n",
'<center>',
'<h3>MSHIP - my status of host IP's</h3>',
'</center>';
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.
| [reply] [d/l] [select] |
|
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?
| [reply] |
|
print join br, map li $_, sort @uphosts;
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.
| [reply] [d/l] [select] |
|
print "<tt>\n";
print "\n<center><h3>MSHIP - my status of host IP's</h3></cente
+r>\n";
my $now=strftime "%m/%d/%Y %H:%M:%S", (localtime);
print "<center>$now</center>\n";
print hr;
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 status of host IPs</h3></center>
<center>$now</center>
<hr />
END
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.
| [reply] [d/l] [select] |
|
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.
| [reply] |
|
| [reply] |
|
|
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.
| [reply] |
Re: improving the aesthetics of perl code
by rlb3 (Deacon) on Jan 21, 2005 at 14:23 UTC
|
Hello,
One of the best things you can do is look at the perlstyle pod if you have a Unix or Unix like system.
From the command like type: perldoc perlstyle.
Or you can find it HERE
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
Original title: 'Re: asthetics' | [reply] |
Re: improving the aesthetics of perl code
by perrin (Chancellor) on Jan 21, 2005 at 15:15 UTC
|
| [reply] |
Re: improving the aesthetics of perl code
by trammell (Priest) on Jan 21, 2005 at 16:08 UTC
|
You might want to read Strunk & White's "Elements of Style", and pay better attention to your grammar, spelling, and punctuation. I find that having to parse someone's nonstandard writing style hinders me from understanding their comments, and thus their code. | [reply] |
|
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.
| [reply] |
Re: improving the aesthetics of perl code
by Ytrew (Pilgrim) on Jan 22, 2005 at 06:20 UTC
|
I didn't intend for this to turn into a style guide, but it seems I've written one. Hopefully, it will prove helpful.
Ytrew's Top Ten Style Points
-
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:
}
}
}
}
or
} # end loop over records from input file
} # end loop over files
} # end read_records()
} # end block of variables private to read_records()
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 | [reply] [d/l] [select] |
|
| [reply] |
|
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
| [reply] |
|
|
|
Re: improving the aesthetics of perl code
by gaal (Parson) on Jan 22, 2005 at 17:57 UTC
|
| [reply] |
Re: improving the aesthetics of perl code
by Anonymous Monk on Jan 21, 2005 at 18:13 UTC
|
In addition to all the other good advice, I have one more thing to say. When you write "Perl" in all caps, it looks like you're shouting. Perl is not an acronym, even though people have had fun making up fakeronyms after the fact. | [reply] |
|
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 :).
| [reply] |
|
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... :-)
| [reply] [d/l] |
|
|
Re: improving the aesthetics of perl code
by jdtoronto (Prior) on Sep 13, 2006 at 18:14 UTC
|
Gee, it's actually not the worst Perl code I have seen by any stretch of the imagination. A couple of points:
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 }
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 | [reply] [d/l] [select] |
|
|