Perl Coding Standards - Wish there were some!

| 2 Comments

Some day some bright light will come forward and lay down the law about how Perl code should be formatted. And, just as likely, they will be roundly ignored, primarily out of sheer spite, but with a grain of "perl lets us do what we want. We don't need no steenkin standards" tossed in just to muddy the argument.

Today's annoyance is cleaning up some long forgotten programmer's way of of formatting blocks of code. Only slightly edited, here's a sub in one of the cgi scripts:

sub _getMediaList( $ $ $ ;$ )
{
  my ($carrierId, $contentType, $contentIds, $formatted) = @_; 

  return undef if (!defined($contentType)); # Charts can have no contents and thus no content type

  my $gpd = GPD::getDbh();

  my @mediaList;
  foreach my $contentId (@$contentIds)
  {
   my $resultList = mediaSearch(dbh => $gpd,
                                carrierId => $carrierId,
                                contentType => $contentType,
                                id => $contentId,
                                showdown => 1); 
   next if (!$resultList->[0]); # Shouldn't happen
   push(@mediaList, $resultList->[0]);
  }

  if ($formatted)
  {
    foreach my $media (@mediaList)
    {   
      $media->{title} = "$media->{artist} - $media->{name} - $media->{album}";
    }   
    return formatMediaListColored('[%s] [%s] [%s] %s [%s] [%s] [%s] [%s]', [ 'id', 'isrc', 'upc', 'title', 'genre', 'label', 'relate
d_content_isrc', 'release_date'], \@mediaList, 1);

  }

  return \@mediaList;
}

Now, I'm not harping on the code itself - this is pretty innocuous, if slightly braindead. But what annoys me is the overuse of newlines to stand off functional blocks. They're not necessary, the indenting is painful, and it makes the code hard to read. How hard is it to rework it like this:

sub _getMediaList( $ $ $ ;$ ) {
    my ($carrierId, $contentType, $contentIds, $formatted) = @_;
    my $gpd = GPD::getDbh();
    my @mediaList;

    return undef if (!defined($contentType)); # Charts can have no contents and thus no content type

    foreach my $contentId (@$contentIds) {
        my $resultList = mediaSearch(dbh => $gpd,
                                        carrierId => $carrierId,
                                        contentType => $contentType,
                                        id => $contentId,
                                        showdown => 1);
        next if (!$resultList->[0]); # Shouldn't happen
        push(@mediaList, $resultList->[0]);
    }

    if ($formatted) {
        foreach my $media (@mediaList) {
            $media->{title} = "$media->{artist} - $media->{name} - $media->{album}";
        }
        return formatMediaListColored('[%s] [%s] [%s] %s [%s] [%s] [%s] [%s]', 
            [ 'id', 'isrc', 'upc', 'title', 'genre', 'label', 'related_content_isrc', 'release_date'], 
            \@mediaList, 1);
    }

    return \@mediaList;
}

To me this is far easier to work with and understand where teh blocks are and how they work together. In my opinion, there's a couple basic rules about code layout:

  • Consistent indenting - 4 spaces or 2, I don't really care which, just use the same thing EVERYWHERE.
  • Curly braces to open a block go on the same line as the introductory keyword (if, foreach, sub, whatever).
  • Lines longer than 80 characters should be broken into sane sub-lines. Just hit enter and indent, folks. Not that hard.
  • Declarations at the top of the block when the variables are used across the entire block.
  • Separate declarations from functional code by a newline.

Now, this particular piece of code has no comments in it, so I'll leave comments for another rant. But I think these basics sure would make perl code a lot more maintainable if everyone at least marginally followed them.


2 Comments

GRIN This is going to be fun...

I agree the indentation leaves something to desire.

The newline set off blocks has been my standard for coding Perl since, oh, 1991 or so. :-) Personally, I find it difficult to read code with cozy opening braces.

Agree on moving variable declarations to the top of the function.

--[Lance]

So actually you're saying "I wish everyone coded my way"? Personally I've seen much worse code layouting than the chunk you included there. To be honest I'd much prefer self-consistency of layout.

Personally I use the "start curlies on a newline" style, mostly..

Have you read "Perl Best Practices"? If its code shared as work, you should be able to implement and explain to people the need for some local style guidelines.

You can also use perltidy on checkout and checkin, and each coder can have their own perltidy settings, and see the code the way they prefer.

Jess

Leave a comment

Pages

OpenID accepted here Learn more about OpenID
Powered by Movable Type 4.23-en

Twitter

Sponsors!

About this Entry

This page contains a single entry by dbs published on April 23, 2009 4:33 PM.

Uncomfortable Plot Summaries was the previous entry in this blog.

Cohousing Day #28 - Superinsulation Works! is the next entry in this blog.

Find recent content on the main index or look in the archives to find all content.