On 2010.05.07 19:37, Steve Bertrand wrote:
> On 2010.05.07 19:02, Harry Putnam wrote:
>
>> It looks pretty awkward, the way I just kind of wedged the default
>> action in there... is there a more canonical way of doing that?
>
> I have to be honest ;)
>
> There was a time a little while back that I decided that I would try
> hard not to troubleshoot code with variables named like this:
>
>> my @ar1 = ("r2one","r2two","r2three","r2four","r2five","r2five","r2four",
>> "r2four","r2three","r2two","r2seven","r2six");
>
> (fwiw, and I don't have time to get into the rest of the code right now,
> I prefer whitespace, so I'd write that as):
>
> my @ar1 = qw (
> r2one
> r2two
> r2three # comment, reader needs info for this var
> r2four
> r2five # more comments
> );
>
> ..etc. However, there's probably another way to do that, but without
> knowing what your variables *mean*, it's too hard to tell.
>
> It may be all well and good when it's confined like this, but even in a
> 50 line script, I'm continuously having to scroll back up to find out
> what it means, and/or where it came from.
>
> I'd recommend a simpler example (as your code screams example... even in
> examples, I found that following your coding practices even in examples
> is *very* beneficial), but use very descriptive names.
>
> Although I've expanded this out of a dispatch table and modified it to
> prevent wrap, it is extremely understandable, even without further context:
>
> my $v4glue = $dns->v4glue();
> my $v6glue = $dns->v6glue();
>
> $self->data({ field => 'v4glue', value => $self->date() }) if $v4glue;
> $self->data({ field => 'v6glue', value => $self->date() }) if $v6glue;
>
> I mean no offence. If anything, use more whitespace. Here's another
> quick rewrite (let the Gods fix it better, as I didn't bother actually
> reading what the code does ;)
>
> my @exp;
> my $r1name = 'r1_r1r1';
> my $arcnt = 1;
>
> for my $element ( @ar1 ) {
>
> push @exp,$element;
>
> $arcnt++;
>
> if (( $arcnt % 4 ) == 0 ) {
>
> dispt( $r1name, @exp );
>
> # my instincts say that $arcnt should be reassigned
> # within this for loop...
s/should/shouldn't/
>
> @exp = ();
> $arcnt = 1;
> }
> }
>
> ...much easier to grok (for me anyway) ;)
>
> Steve
>
--
To unsubscribe, e-mail: beginners-unsubscr...@perl.org
For additional commands, e-mail: beginners-h...@perl.org
http://learn.perl.org/
