Difference between revisions of "Coding Standards"
Jump to navigation
Jump to search
m (Fixed a typo) |
m |
||
| (One intermediate revision by one other user not shown) | |||
| Line 1: | Line 1: | ||
| − | If you commit changes or submit patches to WeBWorK, make sure your work conforms with these standards. (This is not a complete list, just the things that have caused problems in the past.) |
||
| + | Submissions to the WeBWorK code are now subject to coding standards, and pull requests that do not meet these standards will not be merged. |
||
| − | Some of these standards are motivated by readability, some by the line-oriented nature of CVS, some by maintainability. (And some by Sam Hathaway's personal preferences and pet peeves.) |
||
| + | More detail on the standards will be included in this page in the future. For now a good place to start is to make sure you run perltidy on your code. In particular there is a script at $WEBWORK_ROOT/bin/dev_scripts/run-perltidy to do this. You can reference https://github.com/openwebwork/webwork2/blob/main/.perltidyrc to see what this is doing. |
||
| − | == Indentation == |
||
| + | You may need to install a more recent version of perltidy using |
||
| − | |||
| + | <code>sudo cpanm Perl::Tidy</code> to avoid a warning about <code>Unknown option: vxl</code>. |
||
| − | We use the <code>TAB</code> character to indicate a level of indentation, and <code>SPACE</code> characters to line things up in lists. I understand there is some debate as to whether is it appropriate to use "hard" tabs in text files. We do it because some of the WeBWorK developers prefer to view the code with wider indentation (say, eight columns) and some prefer narrower indentation (say, three or four columns). Thus, we use hard tabs for initial indentation and let each developer chose a tab width using their editor. |
||
| − | |||
| − | Here in an example of when to use tabs and spaces. <code>---></code> represents a tab character. |
||
| − | |||
| − | my %errors = ( |
||
| − | --->not_found => "The record was not found.", |
||
| − | --->invalid_id => "The record ID is invalid.", |
||
| − | --->undefined_or_unknown => "The record was undefined or unknown.", |
||
| − | ); |
||
| − | |||
| − | sub bar { |
||
| − | --->my ($self, @recs) = @_; |
||
| − | --->foreach my $rec (@recs) { |
||
| − | --->--->my $Rec = $self->getRec($rec); |
||
| − | --->--->if ($Rec->error) { |
||
| − | --->--->--->warn $errors{$Rec->error}; |
||
| − | --->--->} else { |
||
| − | --->--->--->foo($Rec->a, $Rec->b); |
||
| − | --->--->--->baz( |
||
| − | --->--->--->--->origin => $Rec->origin, |
||
| − | --->--->--->--->destination => $Rec->destination, |
||
| − | --->--->--->--->widgets_per_second => $Rec->widgets_per_second, |
||
| − | --->--->--->--->score => $Rec->score, |
||
| − | --->--->--->--->email_address => $Rec->email_address, |
||
| − | --->--->--->); |
||
| − | --->--->} |
||
| − | --->} |
||
| − | } |
||
| − | |||
| − | As you can see, indentation that is used to "line up" elements on adjacent lines uses spaces, while indentation that is used to indicate some structural hierarchy in the code uses tabs. |
||
| − | |||
| − | == Lining up multiline structures == |
||
| − | |||
| − | We usually use a single additional level of indentation on the trailing lines of a multi-line statement or expression. We typically do not line the continuation of a statement up with the opening brace or paren or bracket that encloses it. Otherwise, a change in the length first line of the statement will require repositioning the following lines, which is a pain. |
||
| − | |||
| − | === Wrong === |
||
| − | |||
| − | $some_long_object->callWithALotOfHashArguments(-heliocoper=>$h, -airplane=>$p, |
||
| − | -collisions=>"resolve", |
||
| − | -unresovable_collisions=>sub{my $e = new E::Collision; |
||
| − | $e->throw; |
||
| − | return 0;}, |
||
| − | -arewedoneyet=>"no", -justonemore=>0); |
||
| − | |||
| − | === Right === |
||
| − | |||
| − | $some_long_object->callWithALotOfHashArguments( |
||
| − | -heliocoper => $h, |
||
| − | -airplane => $p, |
||
| − | -collisions => "resolve", |
||
| − | -unresovable_collisions => sub { |
||
| − | my $e = new E::Collision; |
||
| − | $e->throw; |
||
| − | return 0; |
||
| − | }, |
||
| − | -arewedoneyet => "no", |
||
| − | -justonemore => 0, |
||
| − | ); |
||
| − | |||
| − | (I'm not obsessed with lining up =>'s, but you can if you want. I do like to put spaces on either side of that symbol though, since it binds pretty loosely.) |
||
| − | |||
| − | In the example above, the situation could be improved further by defining the <code>unresovable_collisions</code> subroutine in a separate statement and assigning it to a variable, like <code>$throw_coll_except</code>. |
||
| − | |||
| − | Related to this is something that we're not completely sure about, but it's good to think about: |
||
| − | |||
| − | If we've got a multiline call, block, list, or hash, we usually don't put the first statement or item on the same line as the opening brace/bracket/paren/etc. I think this improved readability, since all the arguments are indented the same amount, and nobody gets "lost" at the end of a long line. It's also better for CVS. Likewise, the closing brace/bracket/paren/etc. usually goes on its own line, and not additionally indented. |
||
| − | |||
| − | However, there are exceptions. If the text leading up to the brace/bracket/etc. isn't very long and encloses a lot of short arguments, we sometimes do start the contents of the structure on the same line. There are some cases where it would take up too many lines otherwise. Or just plain look silly. So, this could be all right: |
||
| − | |||
| − | someFunctionCallWithShortArgs($foo, $bar, $baz, $fubar, |
||
| − | $xyzzy, $fizzy, $silly, \@wakakaka, $lalala, $webwork); |
||
| − | |||
| − | And if you've got hashes going, and you format them like this, you should probably leave out those spaces to save space. I like to keep the space after the comma even here, but I'm not married to it. Here's a real-life example: |
||
| − | |||
| − | CGI::table({-border=>"0", -cellpadding=>"10", cellspacing=>"10"}, |
||
| − | CGI::Tr( |
||
| − | CGI::td(...) |
||
| − | ) |
||
| − | ); |
||
| − | |||
| − | I've got to think about this one some more. Suggestions welcome. |
||
| − | |||
| − | == Comments == |
||
| − | |||
| − | Comments should always be indented as far as the code they are next to. Avoid multiline comments that appear to the right of code. Commented-out code should immediately follow the comment character (<code>#$foo...</code>) while comments should have a space after the comment character (<code># this is...</code>). |
||
| − | |||
| − | Don't freak out with the comment character. Usually, a single <code>#</code> will do it. You should probably never be making huge boxes out of them, and you should never ''ever'' do that within a subroutine. I like to use comments prefixed with <code>#####</code> to separate major sections of code, but you only really need that if the major sections are major enough that a comment prefixed with a single <code>#</code> would get lost. |
||
| − | |||
| − | In general, wrap long comments to 80 characters. You can bend this rule a little bit if you have a comment that's like 90 characters and you don't want to use two lines. |
||
| − | |||
| − | {|border=1 |
||
| − | |- |
||
| − | | |
||
| − | |||
| − | === Wrong === |
||
| − | |||
| − | sub foo { |
||
| − | my ($a, $b) = @_; |
||
| − | my $c = $a * $b; # first, get the |
||
| − | # product of $a and $b. |
||
| − | |||
| − | ################################# |
||
| − | #####now call a function######### |
||
| − | ################################# |
||
| − | callFunc($c); |
||
| − | |||
| − | # this code is no longer needed |
||
| − | # oldFunc($a, \$b); |
||
| − | # $c -= $b; |
||
| − | |||
| − | return $c; |
||
| − | } |
||
| − | |||
| − | || |
||
| − | |||
| − | === Right === |
||
| − | |||
| − | sub foo { |
||
| − | my ($a, $b) = @_; |
||
| − | |||
| − | # first, get the product of $a and $b. |
||
| − | my $c = $a * $b; |
||
| − | |||
| − | # now call a function |
||
| − | callFunc($c); |
||
| − | |||
| − | # this code is no longer needed |
||
| − | #oldFunc($a, \$b); |
||
| − | #$c -= $b; |
||
| − | |||
| − | return $c; |
||
| − | } |
||
| − | |||
| − | |||
| − | |} |
||
[[Category:Developers]] |
[[Category:Developers]] |
||
Latest revision as of 12:45, 6 April 2023
Submissions to the WeBWorK code are now subject to coding standards, and pull requests that do not meet these standards will not be merged.
More detail on the standards will be included in this page in the future. For now a good place to start is to make sure you run perltidy on your code. In particular there is a script at $WEBWORK_ROOT/bin/dev_scripts/run-perltidy to do this. You can reference https://github.com/openwebwork/webwork2/blob/main/.perltidyrc to see what this is doing.
You may need to install a more recent version of perltidy using
sudo cpanm Perl::Tidy to avoid a warning about Unknown option: vxl.
