Guidelines for the zealots ![(smile)](/confluence/s/34gb19/8804/10mvnxf/_/images/icons/emoticons/smile.svg)
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
Perl CodingStyle
Mostly quite sensible Perl stuff as documented in perlstyle:
http://perldoc.perl.org/perlstyle.html
Differences from the standard Perl style are documented below.
Formatting
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).
Braces and Parentheses
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(); }
- (status of this guideline: proposed by DanielQuinlan. everyone happy with it?)
- (+1 – JustinMason)
- (+1 – DuncanFindlay who has finally weened himself off cuddled elses)
One liners
";" should always be at the end of a line unless its a "for", no points for squeezing more code on the same line.
- (status of this guideline: proposed by DanielQuinlan. everyone happy with it?)
- (+1 – JustinMason)
- (+1 – DuncanFindlay)
Function Arguments
Use @_ rather than shift if possible. For example:
sub foo { my ($foo, $bar) = @_;
instead of
sub foo { my $foo = shift; my $bar = shift;
- (status of this guideline: proposed by DanielQuinlan. everyone happy with it?)
- (+1 – JustinMason)
- (+1 – DuncanFindlay - shift should be fine for one argument subroutines, IMO)
Accessors
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.
- (status of this guideline: proposed by JustinMason. everyone happy with it?)
- (+1 – Daniel Quinlan except where we already do this)
Return Values From Functions
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.
- (status of this guideline: proposed by JustinMason. everyone happy with it?)
- (+1 – Daniel Quinlan)
- (+1 – DuncanFindlay)
C CodingStyle
In our C code we took the easiest way and adopted the [http://www.apache.org/dev/styleguide.html Apache Developers' C Language Style Guide].
One addition, arising from [http://issues.apache.org/SpamAssassin/show_bug.cgi?id=4593 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.