Monthly Archives: September 2012

Learning from Bad Code

I’ve written before about Linux Format’s habit of sharing badly written Perl code. I thought things were improving, but in the new edition (November 2012, issue 163) they’re back to their old tricks.

This time it’s a tutorial called “Starfield: Learn new languages”. In this tutorial Mike Saunders writes similar starfield simulation code in C, Python and Perl. Mike’s loyalties are made perfectly clear when these three sections are entitled “Low Level”, “High Level” and “Unusual Level” respectively, so I wasn’t expecting much. But here’s what I got.

#!/usr/bin/perl

$numstars = 100;

use Time::HiRes qw(usleep);
use Curses;
$screen = new Curses;
noecho;
curs_set(0);

for ($i = 0; $i < $numstars ; $i++) {
  $star_x[$i] = rand(80);
  $star_y[$i] = rand(24);
  $star_s[$i] = rand(4) + 1;
}

while (1) {
  $screen->clear;

  for ($i = 0; $i < $numstars ; $i++) {
    $star_x[$i] -= $star_s[$i];
    if ($star_x[$i] < 0) {
      $star_x[$i] = 80;
    }

    $screen->addch($star_y[$i], $star_x[$i], ".");
  }

  $screen->refresh;
  usleep 50000;
}

Let’s be clear here. This code works exactly as Mike intended it to. But if you’re writing sample code for other people to learn from, I think you should be setting the bar a little higher than “works for me”. I think you should should be aiming to show people good quality code that can be easily mainitained. And this code falls well short of that aim.

Let’s look at some of the problems in more detail:

  • No use strict or use warnings. Let’s be generous and assume the editor removed them to save space.
  • Undeclared variables. Of course. if the code contained use strict then the variables would all have had to be declared. Not declaring them means that we’re using package variables rather than lexical variables. In code this simple it doesn’t make a difference. But it’s a bad habit to get into.
  • Indirect object notation. When creating the Curses object the tutorial uses the syntax $screen = new Curses. Again, not a problem in this program, but a really bad habit to be encouraging. In the article’s defence, the documentation for the Curses module only includes this flawed syntax.
  • Split data structures. The author of the article says “instead of having an array of stars containing coordinates and speeds for each one (i.e. an array of arrays), to make things simpler we’ve just set up three arrays.” I read this to mean “I’ve never been able to work out how to make arrays of arrays work in Perl so I’ve taken the easy way out.” This is, of course, a terrible idea. Linked data items should be stored in the same data structure.
  • C-style for loops. The mark of a C programmer who never really got to grips with Perl. The C-style for loop is rarely used in Perl code. The foreach loop almost always leads to more readable code.
  • Magic numbers. The size of the screen and the maximum speed of the stars appear as numbers in the code. Even if you’re not a Perl programmer, surely you would know that it’s good practice to move those into variables or constants.

With all that in mind, here’s my version of the program.

#!/usr/bin/perl

use strict;
use warnings;

use Time::HiRes qw(usleep);
use Curses;

my $numstars = 100;
my $screen_x = 80;
my $screen_y = 24;
my $max_speed = 4;

my $screen = Curses->new;
noecho;
curs_set(0);

my @stars;

foreach my $i (1 .. $numstars) {
  push @stars, {
    x => rand($screen_x),
    y => rand($screen_y),
    s => rand($max_speed) + 1,
  };
}

while (1) {
  $screen->clear;

  foreach my $star (@stars) {
    $star->{x} -= $star->{s};
    $star->{x} = $screen_x if $star->{x} < 0;

    $screen->addch($star->{y}, $star->{x}, '.');
  }

  $screen->refresh;
  usleep 50000;
}

What do you think? Is it more readable? Easier to maintain? Are there any problems or improvements that I’ve missed?

Perl School 3

The second Perl School course is just under four weeks away (and there are still tickets available) but it’s time to start looking ahead.

The third Perl School is going to be on 8th December. Like the others, it will be at Google Campus.  But this time the subject will be slightly different. The first two have been aimed at people who don’t know much Perl, but this one will be the first in a series for Perl programmers who might just not be completely up to date with the latest tools. This one is called “Object Oriented Programming with Perl and Moose”. Tickets are £30 and are available online. More details of the course are on the Perl School web site.

The more observant amongst you will have noticed that I’m giving a two-hour tutorial on Moose at the London Perl Workshop two weeks before this course. That tutorial will cover something like a quarter of the material in the full-day version.