|
I was reading some quotations and I read a lot of them that reflect my thinking.
"When I am working on a problem I never think about beauty. I only think about how to solve the problem. But when I have finished, if the solution is not beautiful, I know it is wrong. "
- Buckminster Fuller (1895-1983)
When programming perl I have the same way of working. I try to solve the problem. Only afterwards I examine if it's ok code.
Analyzing is not really my cup of tea, I just get in there and get the problem solved(I have to say that with every new experience my code gets better). Afterwards I see if the code is good and most of the times I think there are hings that could be imporved.
It's vicious cirkel: starting to solving a new problem I use the knowledge of my previous project to write better code, but afterwards I see other things that could be improved.
What are other opinions on this matter.
I would also bring on as a second issue that analyzing and programming is best not done by one guy. Because or your a good analyzer or you are a good programmer, but being good at both is almost impossible.
This last statement is being said by some guys I work with. I haven't formed a opinion on that issue yet.
My opinions may have changed,
but not the fact that I am right
|
On elegant coding...
by merlyn (Sage) on Oct 12, 2000 at 18:52 UTC
|
"When I am working on a problem I never think about beauty. I only think about how to solve the problem. But when I have finished, if the solution is not beautiful, I know it is wrong. " - Buckminster Fuller (1895-1983)
That quote is so true for my programming style, and it's what distinguishes
the "used for 10 minute hacks" from the "publish it in a column code".
I can't tell you the number of times I've whipped out some snippet for
public consumption, then sat back and stared at it for somewhere between
10 minutes and 10 hours, simply because it wasn't elegant.
(So if you think the code you see is weird, you should have seen
the rejects! {grin})
It makes me mad when I have to use the same expression in two places, for example.
There's just something wrong about that. Or when it looks like it'll
be fragile to changes, or not very reusable, or has too many global variables.
It just looks wrong.
Of course, I have deadlines which I'm always up against, so I publish what I've
got occasionally, but you'll find me using phrases like "sorry this looks
so ugly". Often, after I see the code in print a few months later, I'll go
"doh! I could have done XYZ", because I still work on it in my head. It's why
I've revisited that link checking problem over and over again. I am still
not satisfied at how ugly the logic is in the middle of those, or how hard it is
to do things in parallel. But each time I hack out a new version, I'm more and
more proud of the result.
That's probably one of my objections to objects for small programs. It seems
inelegant, making me write too much code to be done in what I can do tighter.
So my rule is "if you don't write 1000 lines of code in your program,
and you're not subclassing an existing class, don't use objects!". And that
upsets some of the "OO for everything" crowd, but Perl is already a hybrid
(part-OO) language, so get over it. {grin}
And back to elegance: each part of a program should do one thing well. Very well,
in fact. I chunk the subroutines adding whitespace for paragraph breaks.
I try to introduce lexicals as I need them, and then don't let their scope go more
than 10 or 15 lines. In a recent experiment, I wrote the entire top-level
code as Perl pseudo-code, invoking clearly-named subroutines for each of the
steps of the algorithm, and then implemented that, but then stared at the code
for a full day, agonizing over the fact that the same thing appeared in
a couple of the steps. Here... take a look:
{
## main loop
if (cache_is_good()) {
show_cache_and_exit("current");
}
if (cache_is_stale()) {
if (i_am_the_writer()) {
if (i_can_fork()) {
if (i_am_the_parent()) {
show_cache_and_exit("stale");
}
## child does:
be_a_child();
update_cache();
exit 0;
}
## cannot fork, so it's up to me
update_cache();
show_cache_and_exit("current");
}
## I'm not the writer, so show old cache
show_cache_and_exit("stale");
}
## cache is dead
if (i_am_the_writer()) {
update_cache();
show_cache_and_exit("current");
}
## we cannot do anything about a bad cache, so retry
close_cache();
sleep 5;
redo;
}
See, you can probably see the whole algorithm, self described, right here.
But I agonized over the fact that some of those steps are duplicated in a way,
but we get to them because of different conditions. (This is my next
Linux Magazine
column, but it won't be online for a few months, as usual.)
I tried rewriting it, changing the order of testing for some of the conditions, but
it got only messier.
So after a day, this is the main code for my program,
followed by 100 lines of code to implement the subroutines. And even after I had
coded them, I rearranged them so that the subroutines that shared the same global
data lived in their own block so the globals could disappear. This even helped me
explain the program easier, and I know it'll be easier to maintain.
And when I complain about code posted here in the Monestary, it's these
ideals that I'm holding the code against. I'm sorry if I sound annoying about
them sometimes. It's just that I view programming as an art, not just
a hack.
Whew. Long post. Comments?
-- Randal L. Schwartz, Perl hacker | [reply]
[d/l] |
|
|
The novice hesitates and then cautiously responds...
Many of your thoughts strike chords with my personal development style (paraphrasing):
- Planning the implementation using pseudocode comments before writing implementation code
- Minimizing globals
- Not using classes when old-fashioned structured programming will suffice
- Using white space to visually separate sections of code
- Reviewing it after the fact to make sure it's right
However, do you not hold a rather unique position in the community? You are held to a higher standard because your code is often seen as a standard to follow. As a leader, a columnist, and a mentor, you are one of the sources for "how it should be done."
Yes, your code should be elegant as time, energy, and editors will allow.
Yes, our code should as elegant as possible. But, does not the ability for elegance come with experience? I don't expect my daughter's watercolors to match the quality or the simplicity of master artists. So, too, the quality of my perl will not match yours for a period of years (if ever).
As possible is the key. As a leader and mentor, please don't forget that some do not yet have skills that match yours. They have the potential, just not the experience.
In the short time I've been slouching about, I've found perlmonks to be a community interested in helping folks improve their knowledge and skills, as opposed to other communities where knowledge and ideas are hoarded for some questionable tactical advantage. I personally appreciate that very much.
I would ask, though, that "elegance" be gently taught. After all, there are many that need to do their work very quickly, without the luxury of extended research, review, or analysis. A regrettable portion of our positions as programmers is that we're given too little time and resources to do our work properly.
This is, as you well know, why there are so many hacks in "finished" software. One quote that comes to mind, though I confess I cannot properly attribute it, is, "Software is never finished; it's just abandoned."
Also, elegance, to some degree is in the eye of the beholder. For example, I recognize that the standard is to use this:
if (cache_is_good()) {
show_cache_and_ext("current");
}
However, you will almost always find my code expresses it as:
if ( cacheIsGood() )
{
showCacheAndExit( "current" );
}
Does that make my code less elegant? To you, perhaps...however, I find it:
- Clearly denotes the block.
- Is easier to debug when I've forgotten enclosing braces or mismatched my parentheses.
- Is easier to read when viewed or printed with proportional spaced fonts.
- Clearly separates my function calls from built-ins.
That's just my personal preference. It works for me and I do not claim it works for anyone else.
I would hope that in your quest to improve the elegance of our code, you would recognize that and not vote me down because I format my tokens differently that you do.
In my opinion, good programmers continue to look for ways to improve their skills, even though previous efforts cannot (or will not) be revisited. They also recognize that "coding standards" should be guidelines and not straight-jackets. In my opinion, that's where the artistry comes in. | [reply] |
|
|
Seems to me that you're looking to heavily on the brushstrokes
and not enough on the theme when referring to the blocks.
(Disclaimer: This is not a rant on either footpad or
merlyn). What I note from merlyn is that one does not
simply reach a point where everything is elegant. This is
either comforting or a complete disappointment. I'm not sure
which, yet. I appreciate the sentiment; that much is clear.
footpad, the mistake I notice in your post is that you
give a higher credence to merlyn. Possibly more than other
monks, but he is, after all, 'just another perl hacker'. I
guess what I'm saying is that I agree with the overall concept
being discussed here: there is a good quality about treating
code as an art. (Here's the potential fire, but I've tried
my hardest to put the bucket of water right in front of you)
Just because someone is considered a "master" in their field
of art does not mean that their work is good. I think merlyn
is a good artist, but it is because what he does is important
to himself, not that it is important to you.
I don't have a cool quote from anyone famous, but I'm
certain that someone probably said something like this in the
past (Probably Howard Roark): You can't create art for
anyone but yourself. If it happens that your work reaches
others, then they can only take from it what they want to
take from it. This will be dependent on their personal path
that has led to this moment. It will be theirs only, and
even as the artist, you will not have any claim to ownership
of that which they get from your work.
ALL HAIL BRAK!!!
| [reply] |
|
|
|
|
| |
|
I was just scanning some of my code to see how I do it and I notice that if the block contains a single statement, I almost always code like this:
showCacheAndExit("current") if cacheIsGood();
That way it reads the same way I would say it in English. But that's just me and I am FAR from considering myself an experienced Perl dude.
-It's snowing in Mammoth!!! | [reply]
[d/l] |
|
|
In my experience with my own modest Perl
skills I have gradually moved from the "hack-hack-hack-does it work? Yeah!"
style of coding to a more aesthetic appreciation for
the artistic aspects programming.
When I'm working on a project I try to figure out an elegant
solution to the problem, that is, a solution that requires the least
amount of coding, but the implementation itself isn't always very elegant.
Now, I certainly know when I've written something sloppy, and although most the time
I have to go with it due to time constraints, but it doesn't keep me from feeling
dirty somehow. I certainly appreciate
programming elegance even if I'm not capable of it.
I aspire for the day when my programs become elegant both in form and in
function, because I think Merlyn's right: the more artistic a
program is the easier it is to maintain, the easier it is to read,
the better it works.
But for now I am content to color with crayons. I will leave the oil paints
to the Gurus.
Gary Blackburn
Trained Killer
| [reply] |
|
|
I enjoy at least trying to redo these things (whether or not the result is more or less maintainable/elegant is another question). My effort (which I know is probably something like what you already tried):
{
## main loop
show_cache_and_exit("current") if cache_is_good();
unless (cache_is_stale()) {
## cache is dead
update_cache(),show_cache_and_exit("current") if i_am_the_writer();
## we cannot do anything about a bad cache, so retry
close_cache();
sleep 5;
redo;
}
## If I'm not the writer then show old cache
show_cache_and_exit("stale") unless i_am_the_writer();
## If I cannot fork it's up to me
update_cache(),show_cache_and_exit("current") unless i_can_fork();
show_cache_and_exit("stale") if i_am_the_parent();
## child does:
be_a_child();
update_cache();
exit 0;
}
| [reply]
[d/l] |
|
|
| [reply] |
|
|
Excessive !?!?! There's 3 if's and 3 unless's. I'd say they're perfectly
balanced :-) Really, though, I was just trying to get rid of all the
nested blocks, which is sometimes easier to do (in this case anyway, I believe) only after
seeing the code WITH all the nested blocks.
Update:oops, meant to reply to merlyn, and replied to myself instead :-)
| [reply] |
|
|
First of all I agree with the various thoughts and points you make. So let me just comment on the example.
I see what you mean. Aesthetically I don't like the exit condition being woven through the logic, what happens if some day you want to have this cache updating be part of a longer-running script? Also reading the logic through I saw a lot of combinations of actions being taken, but it was not at all obvious why some of them were. And the nesting is pretty darned deep.
I used an indicator variable to do the actions and restructured. Comments?
{
## main loop
my $cache = 'current'; # Hope for the best
if (not cache_is_good()) {
$cache = 'stale'; # Changed expectations
if (i_am_the_writer()) {
if (cache_is_stale()) {
unless (start_background_update()) {
$cache = 'current'; # Gotta do it foreground
}
}
else {
$cache = 'current'; # Have nothing to show, gotta do it
}
}
elsif(not cache_is_stale()) {
# Stuck, gotta retry
close_cache();
sleep 5;
redo;
}
update_cache() if $cache eq 'current'; # Make it so
}
show_cache($cache);
}
sub start_background_update {
# NOTE: Uses implicit returns.
if (i_can_fork()) {
unless (i_am_the_parent()) {
be_a_child();
update_cache();
exit 0;
}
}
}
UPDATE
Yeah, I know. I was using implicit returns without a comment. At the time it made sense, but that is the kind of thing I rethink and mention shortly after... | [reply]
[d/l] |
|
|
I'm not a real big fan of state variables, and in fact, for this example,
I had tossed around doing some status checks similar to what you've done,
but rejected it after a couple of false starts.
The problem with state variables is that they introduce some additional
coupling:
- What if the variable gets set to a value other than the legal values (or not initialized)? You could branch more often or less often than you think.
- What if you refactor and now need a finer-grain distinction (stale vs real stale)?
How do you track down which values to check, or does it now need some disjunctive
tests?
- Tracing the program flow now also requires a data simulation as well.
Yeah, again, just reporting on where I've been burned. But I think I've got
enough scars to not just be a guy in a diner on this one. You know, the guy
at the end of the counter who has an opinion on everything even though
he's not been there? {grin}
-- Randal L. Schwartz, Perl hacker
| [reply] |
|
|
|
|
Well, this code has one inconsistency. In one case, you aggregate an action with an exit, while in another you dont. E.g.,
show_cache_and_exit($STATE);
aggregates two actions, while
update_cache();
exit 0;
would have been written as
update_cache_and_exit(0)
if it were consistent with the aggregation methodology used for show_cache()
Alternatively, and in the favor of fine granularity,
show_cache_and_exit($STATE);
Could be written as
show_cache();
exit($STATE);
| [reply]
[d/l]
[select] |
|
|
Yeah, I played around a little with factoring. You're right, I could have combined those.
But the inelegance was that there are two different reasons we could be updating
the cache in the parent... one because we couldn't fork, and the other because
the cache was so old we dared not show a stale cache. And I was trying to figure
out how to have both of those fall out to the same place in the program without
freaking out with state variables or contorted conditions, and just didn't get there.
-- Randal L. Schwartz, Perl hacker
| [reply] |
|
|
Being a former logic programming daemon, let's take a completely different take on writing this. Among other things, this different take shows that you should use constants instead of strings to avoid spelling errors.
## main loop
if (cache_is_good()) {
show_cache_and_exit("current");
}
# INSTEAD BECOMES
cache(GOOD) && show_cache && exit('current');
if (cache_is_stale()) {
if (i_am_the_writer()) {
if (i_can_fork()) {
if (i_am_the_parent()) {
show_cache_and_exit("stale");
}
## child does:
be_a_child();
update_cache();
exit 0;
}
## cannot fork, so it's up to me
update_cache();
show_cache_and_exit("current");
}
## I'm not the writer, so show old cache
show_cache_and_exit("stale");
}
#### BECOMES
(cache(BAD) && sleep(5) && redo)
||
(cache(STALE) &&
(!i_am_writer && show_cache && exit(STALE))
||
(
i_am_writer &&
(forkable() && (
(parent && show_cache && exit(STALE))
||
(child && update_cache && exit(0)
))
|| (update_cache && exit(CURRENT)
)
Prince "--'s here I come! Where is the preview button for comments? Why is this TEXTAREA so small? I cant see anything?" PAWN | [reply]
[d/l]
[select] |
|
|
I shun this coding style, because it either implies an additional dependency which
doesn't exist,
or introduces a dependency which shouldn't be there.
If you use
a && b && c
where you mean
if (a) { b; c; }
and b returns false, you're hosed.
Therefore, as an element of "elegance", I don't introduce or require any more
dependencies than I can safely justify.
In practical terms, I'd reject your code during a code review, demanding a rewrite
on the grounds of maintainability.
-- Randal L. Schwartz, Perl hacker | [reply]
[d/l]
[select] |
|
|
|
|
RE: Just thinking ...
by mirod (Canon) on Oct 12, 2000 at 18:50 UTC
|
analyzing and programming is best not done by one guy. Because or your a good
analyzer or you are a good programmer, but being good at both is almost impossible.
<rant mode="on">
With all due respect with the poeple you work with this is a load
of BS.
It is very easy to write a perfect design... as long as you
don't have to implement it. Only going through the actual writing
of the code will reveal the flaws in the design. Programming
is the messy part, the part when clean abstractions hit
reality.
I agree that separating the analysis and the programming is a good thing
but saying that you can't be good at both is non-sense. Analysis
must take implementation into account. And analysis needs
to be refined as programming progresses. It is too easy for
the author of a neat but un-implementable design to put the
blame on the darn programmers who can't write what he told
them too.
Programming is not a "detail" as I heard it once!
<rant mode="off">
Ranting aside it is generally a good idea for the designers
to be good coders, and most of all to have to be part
of the coding team. Makes them more responsible about what
they design.
| [reply] |
RE: Just thinking ...
by extremely (Priest) on Oct 13, 2000 at 03:08 UTC
|
In reading the comments already posted, I think we need
a new word other than beauty. Half of these posts stumbled
off-topic a bit to coding-style rather algorithm beauty.
Personally I find merlyn's coding style a bit ugly but
the algorithm is purty. =)
I like what princepawn is doing with his logic stuff
and the constants stuff especially. But I find it even
uglier.
mirod's rant saved me from writing the same thing =)
My dad once said, "There is all kinds of good looking,
but there is only one kind of ugly." Now he was talking about
dogs or cars or something but it is kind of true here.
I see lots of great ways to express the same task, clearly
and in a mostly self-documenting way. And a lot of them just
moved the ugly out of sight. =) But for all those cases,
the algorithm is generally non-ugly. Most of what I see is
just different ways to decorate the dog.
Oddly, I was just working on a Cache module for a nasty
little web problem I have. I have a hidden webserver that
only my public webserver can speak to, generating graphs
on that second machine. In an effort to not beat the hidden
server to death (it has to do real work too) I want to cache
the images locally. Rather than run a cache on the hidden
machine (eek, memory) or a reverse cache on the public
machine (eek only helps with the http and not the other
protocols I have to get working next) I did a the next
best thing. I threw Perl at it. =)
What I got was this:
use DiskCache;
my $xdc = new DiskCache "image", 300;
if ( $xdc->exists( "filename.ext" ) ) {
print $xdc->get( "filename.ext" );
} else {
$xdc->del( "filename.ext" );
my $newdata = GetDataFromSomeWhere();
print $newdata;
$xdc->put( "filename.ext", $newdata );
}
Now that I've seen merlyn's stuff I suppose I'll have
to do it right... =)
--
$you = new YOU;
honk() if $you->love(perl) | [reply]
[d/l] |
|
|
