The CodingStyle used in an application can lead to various heated debates. To avoid those in future, the SpamAssassin team tried to come to a compromise. Not everybody likes every single detail but sometimes one has to bite the bullet to avoid long discussions
Mostly quite sensible Perl stuff as documented in perlstyle:
http://perldoc.perl.org/perlstyle.html
Differences from the standard Perl style are documented below.
The main difference between our style and the standard Perl style is that we use an indentation level of 2 and try to keep everything under 80 columns (79 not counting the newline).
elsif and else are uncuddled, no spaces on the inside of paretheses, and try to avoid one-liner conditionals that require braces. If braces are needed, use multiple lines. Open braces should appear on the same line of the condition, unless it's a multiple line condition. For example:
if ($foo) { do_foo(); } elsif ($bar_really_long_condition_that_lasts_longer_than_40_columns && $foo_really_long_condition_that_lasts_longer_than_40_columns) { do_bar(); } else { do_else(); } |
";" should always be at the end of a line unless its a "for", no points for squeezing more code on the same line.
Use @_ rather than shift if possible. For example:
sub foo { my ($foo, $bar) = @_; |
instead of
sub foo { my $foo = shift; my $bar = shift; |
We don't use perl-style accessors very frequently (ie.
sub foo { my ($self, $val) = @_; if (defined $val) { $self->{foo} = $val; } else { return $val; } } |
Instead, the more wordy Java/C++ style is preferred:
sub get_foo { my ($self) = @_; return $val; } sub set_foo { my ($self, $val) = @_; $self->{foo} = $val; } |
The reason why is detailed at PerlAccessorsConsideredHarmful.
Returns should be explicit, instead of implicit, for clarity:
sub foo { ...stuff...; $val = 1; return $val; } |
instead of
sub foo { ...stuff...; $val = 1; } |
reason: in the latter, $val
is returned, but it's not explicit and not obvious. A later change could result in code being added after the assignment to $val
, since it's not clear that the value is returned by the function.
In our C code we took the easiest way and adopted the Apache Developers' C Language Style Guide.
One addition, arising from bug 4593: if there are warnings about variables being signed/unsigned, caused by use of "int" types in system calls that accept "size_t" (or similar), it is better to *carefully* perform a cast in the call(s) to the specific system calls being warned about, instead of changing the type of variables on a global scale to be a "size_t". Bug 4593 is a good example of how this can cause bugs as a side-effect due to -1 being used as an error indicator.