Mike McClain wrote:
Hi John,
     Thank you for your comments.
     Though I've been coding for many years I've never had a chance to
work in a team environment and thus never had the benefit of code reviews.
It's an education to see things from another viewpoint.

On Sat, Oct 30, 2010 at 03:16:51PM -0700, John W. Krahn wrote:
Mike McClain wrote:
<snip>
use warnings;   #   duplicates -w

It does not duplicate -w, in fact -w has no effect after this point in
your program.  See:
perldoc perllexwarn

Since the first sentence in `perldoc perllexwarn` is:
'The "use warnings" pragma is a replacement for both the command line
        flag -w and the equivalent Perl variable, $^W.'
You are technically correct but IMHO nitpicking. :;)

You must have an older version of Perl, mine reads differently.

But I was referring to the "Backward Compatibility" section where it says:

    4.   If a piece of code is under the control of the "warnings"
         pragma, both the $^W variable and the -w flag will be ignored
         for the scope of the lexical warning.


use integer;
$|++;           #   unbuffered STDOUT

STDOUT still buffered but now autoflushed.

Point taken thank you.

$hi_ascii = 1                           if( @ARGV&&   ($ARGV[0] =~ /h/i) );
Because the match operator returns true or false in scalar context you
could write that as:
my $hi_ascii = @ARGV&&  $ARGV[0] =~ /h/i;

Thank you, hadn't seen that.

(And shouldn't that be 'a' for ASCII and 'h' for hexadecimal?)

No ascii is the default and 'h' for high ascii (128..255)
makes 'x' a logical choice for hex.

ACSII only defines 128 characters (0 to 127).



     sp  del/;
       ^^^^
Since when is the space character a control character?

Never said it was but since space is used all over in the
table for readability 'sp' makes it clearer to me.

????


sub print_ascii
{   my ($mode, $hi) = @_;
             else
             {   printf( "$sep$mode  %s ", $i, chr($i) ); }
                             ^^^^
         {   printf( "|$mode  %s ", $i, chr($i) );
                         ^
One of these printf format strings is not like the other.

Oops, I'm fallible. :)

And why are $sep and @symbols in file scope when they are only used
inside this subroutine?

Because I'm often a bottom up programmer and often stop when it works.
Obviously they should have been inside but the sub came late in the process.

And why is this subroutine *only* used for its side effect instead of
having it return a value?

What you call side effect is to me primary effect.
I could have used sprintf and returned a string but don't see any advantage.

So that instead of writing:

{   print "\n$control_chars\n";
    print "octal table\n";
    print_ascii('o', $hi_ascii);
    print "\n";
    print "hex table\n";
    print_ascii('x', $hi_ascii);
    print "\n";
    print "decimal table\n";
    print_ascii('d', $hi_ascii);
}

You could have written that as:

{   print
        "\n$control_chars\n",
        "octal table\n",
        print_ascii( 'o', $hi_ascii ),
        "\n",
        "hex table\n",
        print_ascii( 'x', $hi_ascii ),
        "\n",
        "decimal table\n",
        print_ascii( 'd', $hi_ascii );
}

And only called print() once.




John
--
Any intelligent fool can make things bigger and
more complex... It takes a touch of genius -
and a lot of courage to move in the opposite
direction.                   -- Albert Einstein

--
To unsubscribe, e-mail: beginners-unsubscr...@perl.org
For additional commands, e-mail: beginners-h...@perl.org
http://learn.perl.org/


Reply via email to