Some coding style question - Perl

Dan Borlovan <danb1974@xnet.ro> writes:
> (*) printing longer html snippets from inside a cgi
>
> # cannot use heredocs because of conditionals
>
> print sprintf(

s/print s//;

> '<table border=0 cellspacing=0 cellpadding=1><tr><td class="border">'.
> '<table border=0 cellspacing=0 cellpadding=10><tr><td class="%s" %s>',
> ($class) ? $class : 'data',
> ($width) ? 'width='.$width : ''
> );

Of course you can use a heredoc.

$class = "data" unless $class;
my $optwidth = $width ? "width=$width" : "";

print <<"HTML";
<table border=0 cellspacing=0 cellpadding=1><tr><td class="border">
<table border=0 cellspacing=0 cellpadding=10><tr><td class="$class" $optwidth>
HTML

> (*) calling functions with many and/or complex arguments
>
> # query too long to fit one line
>
> $res = $dbh->selectall_arrayref('SELECT field1, field2, field3 FROM table'.
> ' WHERE cond1=? AND cond2=? AND cond3=?',
> undef, val1, val2, val3
> );

Temporary variables are your friend. Also, if you use emacs, it can
be convinced to switch to sql-mode for the heredoc[1].

my $sql = <<"SQL";
SELECT
field1,
field2,
field3
FROM table
WHERE
cond1 = ?
AND cond2 = ?
AND cond3 = ?
SQL

$res = $dbh->selectall_arrayref($sql, undef, val1, val2, val3);

> Another issue would be the choice of variable naming (ok, this is also
> a matter of personal taste or corporate policy). Ruling out jokes like
> $a to $z or $thisisaloopcounter, should it be $arg_ptr or $ArgPtr or
> $pArgument? $res or $result? Should one use $_ anywhere but very
> simple pieces of code (you don't want to ask yourself next year "wtf
> does $_ contain here)?
>
> How complex should your structures be - should somebody who uses
> $struct->[$index]->{'key1'}->{'key2'} be shot on sight?

That depends entirely on the problem.

> Are there good complex examples or "coding style" documents?

perldoc perlstyle for a bunch of answers to (some of) these and other
questions.

[1] mmm-mode with these settings:
(setq mmm-mode-ext-classes-alist '((perl-mode nil here-doc)))
(setq mmm-global-mode t)
(mmm-set-class-parameter 'here-doc ':front "<<[\"']?\\([a-zA-Z0-9_-]+\\)")
--
Lars Balker Rasmussen Consult::Perl

Dan Borlovan <danb1974@xnet.ro> writes:

>(*) printing longer html snippets from inside a cgi
>
># cannot use heredocs because of conditionals
>
>print sprintf(
> '<table border=0 cellspacing=0 cellpadding=1><tr><td class="border">'.
> '<table border=0 cellspacing=0 cellpadding=10><tr><td class="%s" %s>',
> ($class) ? $class : 'data',
> ($width) ? 'width='.$width : ''
>);

'print sprintf' is silly, call sprintf() directly. I don't think
there is any right answer about formatting the string, but since you
are using %s placeholders you could use a here-document in fact.

>$res = $dbh->selectall_arrayref('SELECT field1, field2, field3 FROM table'.
> ' WHERE cond1=? AND cond2=? AND cond3=?',
> undef, val1, val2, val3
>)

This is more a question about formatting SQL - I normally put it as a
here-document so it can follow its own indentation rules, and I
imagine that applies to most cases of embedding other languages in
Perl. (cf Inline)

>Ruling out jokes like $a to $z or $thisisaloopcounter, should it be
>$arg_ptr or $ArgPtr or $pArgument?

The perlstyle manual page has something to say about this.

>Should one use $_ anywhere but very simple pieces of code (you don't
>want to ask yourself next year "wtf does $_ contain here)?

There are certain semantic traps when using $_, in particular a lot of
subroutines will change it without documenting that they do so. It is
a matter of opinion whether this outweighs the stylistic advantages -
or whether there are such advantages. On the principle of 'don't
repeat yourself', I think

for ($string) { s/^\s+//; s/\s+$// }

is better than

$string =~ s/^\s+//; $string =~ s/\s+$//;

and more so for a complex expression like $h{foo}{bar}. You might not
want to use $_ for blocks longer than a few lines though, unless
following a common idiom like reading lines from a file.

>How complex should your structures be

As complex as they need to be to model the data you are processing.
If you have complex data, you need complex structures.

>- should somebody who uses $struct->[$index]->{'key1'}->{'key2'} be
>shot on sight?

Not at all, although you might rewrite it as

my $person = $struct->[$index];
my $dad = $person->{father};
my $dad_age = $dad->{age};

if that is easier to understand - which probably depends on whether
$person or $dad will be needed later.

>Are there good complex examples or "coding style" documents?

I'm not aware of any apart from perlstyle.

--
Ed Avis <ed@membled.com>

Ooh, I can smell a heated thread coming up

On Wed, Jan 28, 2004 at 12:08:13PM +0200, Dan Borlovan wrote:
> Unfortunately the manual examples treat simple and pretty obvious cases;
> real life ones are not simple and not obvious. Let's consider a few samples:

The manual does provide a good start. I think everybody should read
perlstyle, not so much to agree with it but as to recognize the
considerations that are there.

Apart from that I think the clearest guideline I know (and usually
agree with) is to keep the indent level low as long as you don't have
to pay too much in vertical space as a result.

As an example, I'd gladly do

die "oh no! we forget the present!" unless $gift;

isntead of:

if (not $gift) {
die "oh no! we forget the present!";
}

In this case, we win two vertical lines, too, so the former (to me) is
almost immediately preferable. But I won't do:

do { $self->call($_) unless $do_not_call{$_} } for @numbers;

because tagging on statement modifiers gets unreadable quickly. That
said, I'd always consider modifying code as the above to a filter,
which despite being much the same thing logically, seems a familiar
idiom to me:

$self->call($_) for
grep { not $do_not_call{$_} }
@numbers;

I'm just used to reading these things backwards. I think it's a useful
skill, because

for (grep { not $do_not_call{$_} } @numbers) {
$self->call($_);
}

Is more cumbersome. (The reason being you have to change directions in
parsing the filter.)

But I am actually quite likely to code this way, too:

for (@numbers) {
next if $do_not_call{$_};
$self->call($_);
}

Especially if I have reason to believe more logic will be added over
time. In this case I'll also consider naming the iterator, though as a
rule I avoid that in trivial loops. (But see my upcoming YAPC talk.)

> (*) printing longer html snippets from inside a cgi
>
> # cannot use heredocs because of conditionals

> print sprintf(
> '<table border=0 cellspacing=0 cellpadding=1><tr><td
> class="border">'.
> '<table border=0 cellspacing=0 cellpadding=10><tr><td class="%s"
> %s>',
> ($class) ? $class : 'data',
> ($width) ? 'width='.$width : ''
> );

CGI programming often is awful because of these things, yes. That's why
I avoid it :P

But seriously, try the Template Toolkit or HTML::Mason. And if you do
need to code HTML inside real CGI code, you can use interpolation a
little:

print << "EOF";
<table border=0 cellspacing=0 cellpadding=1><tr><td
class="border"><table border=0 cellspacing=0 cellpadding=10><tr><
td class="@{[ $class || 'data' ]}"
@{[ $width ? 'width='.$width : '' ]}>
EOF

> (*) calling functions with many and/or complex arguments

Don't.

If it's your API, always optimize against it. Pass args by name
and never by position.

If it's not your API, well, find the least painful way:

> # query too long to fit one line
>
> $res = $dbh->selectall_arrayref('SELECT field1, field2, field3 FROM table'.
> ' WHERE cond1=? AND cond2=? AND cond3=?',
> undef, val1, val2, val3
> );

There are *two* things you might want to do here. One is for output of
the SELECT and is more easily and widely helpful, the other is for the
pameters and depends a lot on what your query looks like.

If the WHERE is a simple conjunction of several conditions...

my %conditions = (...);
$sth = $dbh->prepare('SELECT field1, field2, field3 FROM table WHERE '.
(join ' AND ', map { "$_=?" } sort keys %conditions),
undef)
$res = $sth->execute(
map { $conditions{$_} sort keys %conditions);

Not a very big win, perhaps, but you can factor it out to a function.
My better tip for you is what to do with field1, field2 etc.:

# possibly the only useful place for Perl's \() syntax!
$rv = $sth->bind_columns(\($field1, $field2, $field3));

This makes much more sense when they are filled with applicative labels:

$rv = $sth->bind_columns(\($age, $**** $location));

Or again if you like automating these kind of things:

my @what = qw(age sex location);
my %row;
$sth = $dbh->prepare('SELECT '. (join',', @what) ." FROM....");
$sth->execute;
$rv = $sth->bind_columns( \(@row{@what}) );
while ($sth->fetch) {
# %row is populated
}

This is very fast, btw.

> Another issue would be the choice of variable naming (ok, this is also a
> matter of personal taste or corporate policy). Ruling out jokes like $a to
> $z or $thisisaloopcounter, should it be $arg_ptr or $ArgPtr or $pArgument?
> $res or $result? Should one use $_ anywhere but very simple pieces of code
> (you don't want to ask yourself next year "wtf does $_ contain here)?

I use $this_kind of variables, but if everybody on my project prefers
something else, it's the thing I'll most easily let go of.

$_ should not be used where it's hard to make sense what it's there for,
but unfortunately, a programmer's concentration tends to waver so what's
obvious ten minutes after coffee might be less clear later.

But sometimes $_ makes code clearer, /davka/ when it's not visible at
all. This is when it's being used implicity for reads and writes both:

while (<>) {
s/^#.*//;
next unless /\S/;
chomp;

($key, $val) = split /=/;
}

Okay, this is idiomatic, but the *reason* it's idiomatic is because it's
so clear.

> How complex should your structures be - should somebody who uses
> $struct->[$index]->{'key1'}->{'key2'} be shot on sight?

Hmm. Deep is usually not as bad as cross-eyed parametrics:

$struct->[ $indexes{ $current{type} } ]{key1}{key2};

It wouldn't be that bad if it were straight defererencing all the way
down *or* the expression had stopped after the first complication. So
I'd rewrite the above as either:

my $index = $indexes{ $current{type} };
$result = $struct->[$index]{key1}{key2}; # your expression, really

Or:
my $element = $struct->[ $indexes{ $current{type} } ]; # deep breath
$result = $element->{{key1}{key2};

Depending on context.

> Are there good complex examples or "coding style" documents?

I don't know of any. The prime risk of them is they tend to be used
dogmatically by people who don't really understand them. Style is always
a guideline, after all.

What I would suggest is picking (say) five big names from the Perl
world, and looking at their code in CPAN. You'll find they aren't always
pretty, by the way, and I'm *sure* their styles will vary. Choose one
that makes sense to you. If you work in a team let it sink in and write
your own style guide. When you're finished, let us know about it!

--
Gaal Yahas <gaal@forum2.org>
http://gaal.livejournal.com/

[A complimentary Cc of this posting was sent to
Ed Avis
<ed@membled.com>], who wrote in article <l1wu7bkfrt.fsf@budvar.future-i.net>:
> >print sprintf(
> > '<table border=0 cellspacing=0 cellpadding=1><tr><td class="border">'.

> 'print sprintf' is silly

.... unless $/ is set.

Hope this helps,
Ilya

Dan Borlovan <danb1974@xnet.ro> writes:

> # query too long to fit one line

> $res = $dbh->selectall_arrayref('SELECT field1, field2, field3 FROM table'.
> ' WHERE cond1=? AND cond2=? AND cond3=?',
> undef, val1, val2, val3
> );

Several people have recommended heredocs for this. The version of XEmacs
I use with the version of cperl-mode that I use isn't always that happy
with heredocs, so I've ended up just doing something like this:

my $sql = "
select field1, field2, field3
from table
where cond1=$val1
and cond2=$val2
and cond3=$val3";
$res = $dbh->selectall_arrayref($sql);

In other words, just using a multiline quoted string and interpolating the
values directly in (although if you're doing the same query multiple times
with different values, you want to go back to using placeholder ?.

--
#!/usr/bin/perl -- Russ Allbery, Just Another Perl Hacker
$^=q;@!>~|{>krw>yn{u<$$<[~||<Juukn{=,<S~|}<Jwx}qn{<Yn{u<Qjltn{ > 0gFzD gD,
00Fz, 0,,( 0hF 0g)F/=, 0> "L$/GEIFewe{,$/ 0C$~> "@=,m,|,(e 0.), 01,pnn,y{
rw} >;,$0=q,$,,($_=$^)=~y,$/ C-~><@=\n\r,-~$:-u/ #y,d,s,(\$.),$1,gee,print

First, thanks for all the suggestions, some of them made me think "damn, why
haven't I though about it myself". Others make me raise more questions. Feel
free to stop me if I become too boring or silly.

Sorry about print sprintf, had somewhere in a dark past (while talking to a
sybase server via ctlib) some problems with printf.

As a side note, I personally prefer not to use a too perlish syntax, partly
because I'm not used to easily read it and partly because sometimes syntax
higlight does not keep up with it (and no I'm not using emacs).

Now to the questions:

* How good is to spread my declarations all over the function? It's certainly
nicer than starting it with a bunch of my($var1, $var2, ...), but you risk
trying to redeclare the same variable.

* This is kind of silly, but giving syntax highlight isn't it more readable to
use 'param='.$value than "param=$value"?

* Back to CGI, how do you format something like this?

print '<some html tags here>',
start_form(-target => $target),
'<more html tags here>,
submit(-name => 'submit', -label => 'Continue'),
'<again some html>',
end_form()
;

Dan

Russ Allbery wrote:

> my $sql = "
> select field1, field2, field3
> from table
> where cond1=$val1
> and cond2=$val2
> and cond3=$val3";
> $res = $dbh->selectall_arrayref($sql);

Keep in mind that if you do not $dbh->quote your variables, you enter the
world of sql injection.

Quote from DBI man:

Using placeholders and @bind_values with the "do" method can be
useful because it avoids the need to correctly quote any variables
in the $statement.

Dan

On Thu, Jan 29, 2004 at 10:46:03AM +0200, Dan Borlovan wrote:
> As a side note, I personally prefer not to use a too perlish syntax, partly
> because I'm not used to easily read it and partly because sometimes syntax
> higlight does not keep up with it (and no I'm not using emacs).

(vim has a fair syntax hilighter for Perl.)

> * How good is to spread my declarations all over the function? It's
> certainly nicer than starting it with a bunch of my($var1, $var2, ...), but
> you risk trying to redeclare the same variable.

Redeclaring a variable issues an optional warning: run with -w/use
warnings and you'll see it.

> * This is kind of silly, but giving syntax highlight isn't it more readable
> to use 'param='.$value than "param=$value"?

That depends on your syntax highlighter.

Some do recognize interpolation taking place.

> * Back to CGI, how do you format something like this?
>
> print '<some html tags here>',
> start_form(-target => $target),
> '<more html tags here>,
> submit(-name => 'submit', -label => 'Continue'),
> '<again some html>',
> end_form()
> ;

You could use the @{[ ]} syntax to interpolate here, too. (You'll find
here-docs useful again, too.)

--
Gaal Yahas <gaal@forum2.org>
http://gaal.livejournal.com/

Dan Borlovan wrote:

> First, thanks for all the suggestions, some of them made me think "damn,
> why haven't I though about it myself". Others make me raise more
> questions. Feel free to stop me if I become too boring or silly.

You know how in university, your professors told you there was no such
thing as a "bad" question? They lied. A bad question is one that
shows no sign of intelligence.

That said, I've not seen a bad question, or bad answer for that matter,
in this thread. So don't worry about being boring or silly. That's
what the "next" button is for ;->

> As a side note, I personally prefer not to use a too perlish syntax,
> partly because I'm not used to easily read it and partly because sometimes
> syntax higlight does not keep up with it (and no I'm not using emacs).

I tend to go with the other philosophy: fix the syntax hiliting ;-)

> Now to the questions:
>
> * How good is to spread my declarations all over the function? It's
> certainly nicer than starting it with a bunch of my($var1, $var2, ...),
> but you risk trying to redeclare the same variable.

Very good. That risk is exactly why you want to spread your
declarations out. Or at least one of the reasons. If you try to
redclare the same variable, the compiler will complain. This is good
because then you know you're reusing the same variable, or you may
decide you do *not* want to reuse that variable and choose a new name
for it.

Another reason is that you don't need to scan the file as far to find
the scope of a variable in case you're trying to figure out why it is
showing up as "15" when you expected it to be "undef". ;-)

> * This is kind of silly, but giving syntax highlight isn't it more
> readable to use 'param='.$value than "param=$value"?

Point of preference, I suppose. They both do exactly the same thing
under the covers inside the perl virtual machine. I'll use either,
depending on which I feel conveys the meaning I'm trying to get across
to the (human) reader.

> * Back to CGI, how do you format something like this?
>
> print '<some html tags here>',
> start_form(-target => $target),
> '<more html tags here>,
> submit(-name => 'submit', -label => 'Continue'),
> '<again some html>',
> end_form()
> ;

Personally, I format this type of thing using the formatting as
recommended by HTML::Template. Others swear by Text::Template.
Regardless, I suggest finding your favourite templating solution,
preferably pre-invented and put on CPAN, and using it. Even though
some of these can help keep "logic" and "view" separate enough to allow
an HTML designer to write the template and the programmer to write the
logic, I find that it helps me keep everything in my mind properly even
though I perform both roles.

That said, I allow my editor (http://fte.sf.net) to do my formating:

print '<some html tags here>',
start_form(-target => $target),
'<more html tags here>', # missed the single-quote ;->
submit(-name => 'submit', -label => 'Continue'),
'<again some html>',
end_form();

Of course, I also have written alot of the formatting code for this
editor... ;-)