Re: First Perl Program

From: J. Romano (jl_post_at_hotmail.com)
Date: 11/20/04 

Date: 20 Nov 2004 09:03:09 -0800

parkerdg@gmail.com (DGP) wrote in message news:<8752e64b.0411161304.440a50ec@posting.google.com>...
>
> I decided Perl was the best tool for the job. After reading a begginer
> Perl book, I was able to complete the program below within a few
> hours. I think this speaks to Perl's ease of use, power, and
> flexibility.
>
> It seems to work pretty well, but I wanted to get input to see if it
> could be improved.

   Wow! Great program for your first! I wouldn't be able to tell it
was your first program just by looking at it. I especially liked the
way you used "warnings" and "strict". Most beginning Perl programmers
don't use them, and in my experience, many advanced Perl programmers
don't either, which is a shame. "warnigs" and "strict" catch so many
errors that it's sad to see so many programmers who are too lazy to
put them in.

   I do want to make three suggestions, however:

   The first one is of your line:

        @parts = split (",",$line);

The documentation for "split" ("perldoc -f split") says that the
split() function takes a /PATTERN/ (and not a string) for its first
argument. That means that your line would be better written as:

        @parts = split(/,/, $line);

Most of the time Perl knows what you mean when you supply a string
instead of a pattern (and in your case it won't make a difference).
However, it's not good to assume that Perl will always know what you
mean. For example, the following two lines are NOT equivalent, even
though they look like they should be:

      @tokens = split(" ", "dog horse cat bird");
      @tokens = split(/ /, "dog horse cat bird");

   My second suggestion to you is to declare the @parts array inside
the:

      if ($line =~ /^116/) {
         ...
      }

loop instead of near the top of your program. And the $line variable
should be declared in the foreach declaration. That is, the foreach
declaration should look like:

      foreach my $line (<INF>) {

The reason for these changes in declaration is that their position
will now let the code maintainer know that @tokens and $line have no
use outside of the blocks they are declared in. But if they are
declared at the top of the program, then that means they could
possibly be used anywhere in the file.

   This is a readability issue, however. If you think that the code
is more readable with the variables declared at the top of the script,
then by all means leave them there. But I'm just letting you know
that if you change them to be declared only inside the blocks (or more
appropriately, the scope) that they are used in, it will be easier for
others to deduce the purpose of what those variables are for.

   My last suggestion is one that I learned after years of coding
experience. When you wrote:

    # Find lines defining points (Type=116)
    if ($line =~ /^116/) {
        @parts = split (",",$line);
        ...
    }

it was very good that you included that comment to find type 116
lines. However, I recomment adding a comment line or two showing an
example of what type of line you have found as the first line of the
"if" block. In other words, I recommend making your "if" block look
like this:

    # Find lines defining points (Type=116)
    if ($line =~ /^116/) {
        # Now $line should look something like this:
        # 116,blue,$19.95,45.5N,63.0W
        @parts = split (",",$line);
        ...
    }

That way, if a maintainer of your code (or you, in a few months) has
to read your code (and not have any input available), it will be
obvious to you/him/her what the input should look like and what the
entries in @parts should look like.

You can also adopt a similar strategy towards the output. Instead of
just:

        # Write out point x,y,z location.
        print OUTF "@parts[1..3]\n";

You can also add a comment line that shows an example of what could be
printed out, like this:

        # Write out point x,y,z location.
        # Example output: blue $19.95 45.5N
        print OUTF "@parts[1..3]\n";

It helps if the example output line would correspond to the example
input line given above.

   An example of the output isn't as important as an example of input,
since a person knowledgeable in Perl should be able to figure it out
relatively easily. But it still helps, espcially if processing the
input is large and complicated.

   Of course, adding these comments won't affect the correctness of
your code, but giving an example of the input you are expecting (and
an example of the output you are writing out) is a great help to
anyone who has to read your code (even if it is you), in case the
input format should ever change or someone needs to review your code
to see how something is done (which can definitely happen if your code
works well and is quite useful).

   There have been times in the past when I've looked back at my own
code that I wrote a year before and been angry at myself for not
including a sample line of the input I was processing. Conversely,
there have been times when I was happy at myself for including a
sample line or two that helped me understand what exactly the split()
function was operating on.

   Overall, I recommend changing your split() function so that it
takes a /PATTERN/ (regular expression) as the first parameter, and
that you include a comment containing an example of a line of input to
process.

   Great code, though! (I wish more long-time coders wrote Perl code
like you do!)

   -- Jean-Luc

Relevant Pages

  • Re: DBI selectall_hashref
    ... smallest applicable lexical scope unless you have a positive reason to ... For Perl this means that most of the time the declaration of scalars ... if you leave it too long you may never adjust ...
    (comp.lang.perl.misc)
  • Re: grabbing array vlaues in a loop where array is redefined
    ... For Perl this means that most of the time the declaration of scalars ... should be combined with the first assignment. ... may never adjust and may mutate into a bitter and twisted troll. ...
    (comp.lang.perl.misc)
  • Re: HTML parsing
    ... > I am completely new to Perl and am needing some help with a short script I ... Your text callback can then sent a flag to tell the link callback to ... You are suffering from preature declaration again. ...
    (comp.lang.perl.modules)
  • Re: Static Typing in Python
    ... >>what I wanted to stress was that it's the explicit ... > declaration, and yet Perl is not statically typed. ... > think there is enough evidence in existing Python ...
    (comp.lang.python)
  • Re: Ada Popularity: Comparison of Ada/Charles with C++ STL (and Perl)
    ... Python get's my vote for most readable in this case. ... > That's actually my biggest problem with Perl. ... to improve code readability, ... > programmer, who wasn't completely confused by their ...
    (comp.lang.ada)