Re: a simple exercise in readability

I'd rewrite as follows:

#!/usr/bin/perl -l

use strict;
use warnings;

use Getopt::Std;

our ($opt_s);
getopts('s');

# get the range boundaries
my $range_start  = ( $opt_s ) ? $ARGV[0] : $ARGV[0] + 1;
my $range_end    = $ARGV[1];

# compute the sum of the range
my $range_length = $range_end - $range_start + 1;
my $range_sum    = ($range_start + $range_end) * $range_length / 2;

print $range_sum;
[download]

The named variables make it easier to tell what it's doing. I might also consider handling the -s option like this:

my $range_start = $ARGV[0];
$range_start++ if ( $opt_s );
[download]

It's a little cleaner, but I think the ternary is actually clearer here. It's more obvious that there's a decision, and what it means.

Ordinarily I'd say it should have comments too, but I'm so enamored with the readability of my own code, I think it doesn't need them anymore. If there were more options, they'd definitely need documentation.

For bonus points, tell me how you might modify the program for usability purposes. For instance, how (if at all) would you handle improper input such as non-integer arguments (e.g. floating point numbers like 2.7)?

I'd probably just put in a check early on and die with a usage statement:

if (    $ARGV[0] !~ m{ \A [+-]? \d+ \z }xms
     || $ARGV[1] !~ m{ \A [+-]? \d+ \z }xms ) {
    die "usage: $0 [-s] <integer> <integer>\n";
}
[download]

A longer, more detailed usage message would be better, but you get the idea.

Comment on Re: a simple exercise in readability Select or Download Code

Replies are listed 'Best First'.

Re^2: a simple exercise in readability
by apotheon (Deacon) on Jan 15, 2007 at 15:07 UTC

I quite like your use of the ternary operator there. I tend to be reluctant to use it because of the usual increased readability of explicit if/else statements (at least in most of the code that I write), though in this case I think it is well suited to the task.

I think there's a limit to the value of explicit step-by-step variable assignment and use, however, for readability. In this case, the arithmetic formula used to produce results seems obfuscated by being broken up the way you've done so -- though it may well just be a matter of taste, or I may just be weird to think it's more readable to keep the formula a little more integrated.

I'd appreciate it if you'd have a look at the relevant parts of my response to liverpole, and perhaps comment on the way I used explicit variable names for the start and end integers, adjusting perhaps for the use of the ternary operator.

In fact, I'll cut out the middleman and provide a version of the relevant snippet using the ternary operator:

my $start = ( $opt_s ) ? $ARGV[0] : $ARGV[0] + 1;
my $end = $ARGV[1];

print( ($end - $start + 1) * ($start + $end) / 2 );
[download]

edit: code typo pointed out by kyle at roughly the same instant I noticed it myself

print substr("Just another Perl hacker", 0, -2);
- apotheon
CopyWrite Chad Perrin

[reply]
[d/l]

Re^3: a simple exercise in readability

by kyle (Abbot) on Jan 15, 2007 at 16:59 UTC

I like things broken up and named. For a relatively simple operation like this, it doesn't make as much of a difference. Yours is shorter and still fairly easy to understand (easier with a comment). I'd say it's a matter of taste.

One thing that might recommend my version over yours is a bug that we both had (I've edited mine since), namely the range length should be ($end - $start + 1). I might say that mine (with the name) makes this bug more obvious. Then again, maybe not. Someone might read the variable name and not the formula, and just assume it's right when it's not.

[reply]
[d/l]