Context Navigation

CodingStyle

Timestamp:: Mar 21, 2010, 11:27:45 AM (16 years ago)
Author:: loomis
Comment:: --

Legend:

: Unmodified
: Added
: Removed
: Modified

Development/Code/CodingStyle

-              v1
+              v2
 There are lots of documents motivating this, feel free to review
 them. [[http://www.kroah.com/linux/talks/ols_2002_kernel_codingstyle_paper/codingstyle.ps In short]]:
+them. [http://www.kroah.com/linux/talks/ols_2002_kernel_codingstyle_paper/codingstyle.ps In short]:
 * I want to read your code.
 …
 === Further reading ===
 * [[http://perldoc.perl.org/perlstyle.html Perl coding style guide]]
 * [[http://www.kroah.com/linux/talks/ols_2002_kernel_codingstyle_paper/codingstyle.ps Documentation/CodingStyle and beyond]], by Greg Kroah-Hartman
+* [http://perldoc.perl.org/perlstyle.html Perl coding style guide]
+* [http://www.kroah.com/linux/talks/ols_2002_kernel_codingstyle_paper/codingstyle.ps Documentation/CodingStyle and beyond], by Greg Kroah-Hartman
 == The basics ==
 …
 === Use meaningful names for globals, short names for locals ===
 <tt>i</tt> is a perfectly valid identifier for a loop variable. However, you
+`i` is a perfectly valid identifier for a loop variable. However, you
 deserve nasty punishment if you use it as a function name.
 On the opposite, <tt>this_is_a_temporary_counter</tt> is plain bad for
+On the opposite, `this_is_a_temporary_counter` is plain bad for
 a loop variable.
 So, some good examples:
+{{{
  sub write_user_credentials
+ {
      # ...
+ }
+}}}
+{{{
  for my $i (0..10) {
      # ...
+ }
+ }}}
 And very bad examples:
+{{{
  sub wuc
+ {
      # WTH???
+ }
+}}}
+{{{
  for my $counter_from_0_to_10_included (0..10) {
      # Excuse me???
+ }
+ }}}
 === Be modular ===
 …
 Here are the classic metrics for modularity:
+* No lines longer than 80 columns.
+** Split or rearrange longer strings
+** Split longer statements
+* If you have more than 3 levels of indentation, split your block
+* If your function goes beyond the screen, split it
+** And don't try to reduce the font. ;)
+* If you have more than 7 local variables, split your function.
+** Sometimes it's OK to have 10, but if you have 15 your code is a problem.
+ * No lines longer than 80 columns.
+   * Split or rearrange longer strings
+   * Split longer statements
+ * If you have more than 3 levels of indentation, split your block
+ * If your function goes beyond the screen, split it
+   * And don't try to reduce the font. ;)
+ * If you have more than 7 local variables, split your function.
+   * Sometimes it's OK to have 10, but if you have 15 your code is a problem.
 === Don't use magic numbers  ===
 The <tt>constant</tt> pragma will give you meaningful names for any
+The `constant` pragma will give you meaningful names for any
 values other than 0 and 1 you need. They'll help you to understand why
 you chose such values on the past.
 Good example:
+{{{
  use constant PI => 3.141592;
+}}}
+{{{
  my $circle_area = $radio * PI * PI;
+}}}
 And the bad example
+{{{
  # Oops! I missed a decimal somewhere!
  my $circle_area = $radio * 3.14159 * 3.141592;
+}}}
 === Comments ===
 …
 Bad examples:
+{{{
  ############################################################
+ #
 …
  ############################################################
  $i++;
+}}}
+{{{
  sub do_something
+ {
 …
     ...
+ }
+}}}
 These are best done like this:
+{{{
  $i++;
+}}}
+{{{
  # Performs task foo...
  sub foo {...}
 …
  # Performs task baz, which is quite similar to bar
  sub baz {...}
  # Performs a set of tasks, returning blah blah with arguments
  # $arg1:
 …
      baz (@args);
+ }
+}}}
 Finally, don't put any credits on your comments, excepting on file
 headers.
 === Don't use <tt>vars</tt> ===
+=== Don't use `vars` ===
 This pragma has been deprecated since Perl 5.6, and that's a long time
 ago. Instead, use the <tt>our</tt> declaration for package-wide
+ago. Instead, use the `our` declaration for package-wide
 variables:
 Good:
+{{{
  our @ISA = ...;
+}}}
 Bad:
+{{{
  use vars qw (@ISA);
  @ISA = ...;
+}}}
 === Curly bracket position ===
 …
 line as the sentence they belong to and close them on a line for their
 own, excepting when it's an else or a do-while block:
+{{{
  if ($foo) {
      ...
+ }
+}}}
+{{{
  while ($bar) {
      ...
+ }
+}}}
+{{{
  if ($foo) {
      ...
 …
      ...
+ }
+}}}
+{{{
  do {
      ...
  } while ($bar);
+}}}
 This way it is perfectly clear where each block starts, finishes and
 …
 function. They should be on a different line, and have nothing else on
 the same line:
+{{{
  sub foo
+ {
      my @args = @_;
+ }
+}}}
 The reason for this is that decent editors (f.i, Emacs) are aware that
 …
 Use them a lot. When in doubt, use them. Especially:
 * Always use parenthesis on function calls
 * Always use them on function calls even when there are no arguments
 The reason is that:
+{{{
  foo();
+}}}
 Is easier to understand than:
+{{{
  foo;
+}}}
 === Prefer methods over plain functions ===
 Especially when writing components, you want to log unexpected
 things. It's free to have a <tt>$self</tt> object, and let it log.
+things. It's free to have a `$self` object, and let it log.
 Only use plain functions if you want them exported to some other
 …
 === Running commands ===
 Do not use backticks or <tt>system</tt>, nor use <tt>open</tt> for pipes. The
 <tt>CAF::Process</tt> module has all you need, and it won't spawn new
+Do not use backticks or `system`, nor use `open` for pipes. The
+`CAF::Process` module has all you need, and it won't spawn new
 subshells, which is much safer.
 Another advantage of the <tt>CAF::Process</tt> module is that your
+Another advantage of the `CAF::Process` module is that your
 command line will be automatically logged at verbose level, which
 saves you snippets like this:
+{{{
  $self->debug (5, "Going to run ls -l");
  system ("ls", "-l");
+}}}
 This will become inconsistent with different uses. Instead, do:
+{{{
  # Or any reporter object.
  my $proc = CAF::Proces->new (["ls", "-l"], log => $self);
 …
  # One-line version:
  CAF::Process->new (["ls", "-l"], log => $self)->run();
+}}
 The log option is any CAF::Logger object, for instance the component
 …
 an encrypted password to usermod. In this cases, you don't want your
 command logged. Just don't pass any log argument to CAF::Process::new:
+{{{
  my $proc = CAF::Process->new (["ls", "-l"]);
  $proc->run();
+}}}
 ==== Examples ====
 …
 Create the contents to be piped:
+{{{
  my $contents = "Hello, world";
+}}}
 Create the command, specifying $contents as the input, and "execute" it:
+{{{
 my $proc = CAF::Process->new (["cat", "-"], stdin => $contents);
 $proc->execute();
+}}}
 ===== Piping in and out =====
 …
 Suppose we want a bi-directional pipe: we provide the command's stdin,
 and need to get its output and error:
+{{{
  my ($stdin, $stdout, $stderr) = ("Hello, world", undef, undef);
  my $proc = CAF::Process->new (["cat", "-"], stdin => $stdin,
 …
                                 stderr => \$stderr);
  $proc->execute();
+}}}
 And we'll have the command's standard output and error on $stdout and
 …
 Create the command:
+{{{
  my $proc = CAF::Process->new (["ls", "-lh"]);
+}}}
 And get the output:
+{{{
  my $output = $proc->output();
+}}}
 === File handling ===
 …
 that you should be aware of. For instance, the following code is an
 example of what shouldn't be done:
+{{{
  open (FH, ">/tmp/foo");
+If /tmp/foo already exists and is a symbolic link to /etc/shadow, you
+}}}
+If `/tmp/foo` already exists and is a symbolic link to `/etc/shadow`, you
 just lost all accounts on your system.
 …
 Don't open files directly. Use the CAF::FileWriter module to do the
 work for you:
+{{{
  my $fh = CAF::FileWriter->open ("/my/file", log => $self);
  print $fh "Hello, world!\n";
  $fh->close();
+}}}
 This module will perform all security checks for you, and will log
 …
 You can specify the permissions you want at instantiation, too:
+{{{
  my $fh = CAF::FileWriter->open ("/my/path", mode => 0400, log => $self );
  print $fh "Hello, world!\n";
  $fh->close();
+}}}
 If you find any errors and have to discard your contents, just call
 the cancel method, and the existing file will be preserved:
+{{{
  my $fh = CAF::FileWriter->open ("/my/path", mode => 0400, log => $self);
  print $fh "Hello, world!\n";
 …
+ }
  $fh->close();
+}}}
 Finally, if the contents you generate are the same as on the original
 …
 Sometimes you just want to add a couple of lines to an existing file,
 without destroying its current contents. Or to remove them. The module
 <tt>CAF::FileEditor</tt> is just for that. It is a subclass of
+<tt>CAF::FileWriter</tt>, and when you call the <tt>string_ref</tt>
+`CAF::FileEditor` is just for that. It is a subclass of
+`CAF::FileWriter`, and when you call the `string_ref`
 method you'll get a reference to the file's contents so that you can
 freely update them.
 …
 For instance, you only want to change the first "a" for a b on the
 first line of a file:
+{{{
  my $fh = CAF::FileEditor->open ("/foo/bar",
                                  log => $self);
 …
  $$ref_to_contents =~ s{a}{b};
  $fh->close();
+}}}
 If you want to add text at the beginning of the file, the
 <tt>head_print</tt> method is what you are looking for.
+`head_print` method is what you are looking for.
 ==== Other tasks with files ====
 When in doubt, use <tt>LC::</tt> modules and functions, over standard ones. For instance, use <tt>LC::File::copy</tt>, <tt>LC::File::move</tt> instead of <tt>File::Copy</tt> and friends.
 <tt>LC::</tt> functions perform more security checks and are less prone to symlink attacks.
+When in doubt, use `LC::` modules and functions, over standard ones. For instance, use `LC::File::copy`, `LC::File::move` instead of `File::Copy` and friends.
+`LC::` functions perform more security checks and are less prone to symlink attacks.
 == Input handling ==
 …
 Also, when you print files that will be sourced by shell scripts, be
 sure to print all values between single quotes. This is true for
 almost every file you have under /etc/sysconfig.
+almost every file you have under `/etc/sysconfig`.
 == Quality of messages ==
 …
 === Don't annoy the user ===
 The main principle is not to annoy the user. Use <tt>info</tt> and
 <tt>ok</tt> methods only for '''very''' important messages. A small
+The main principle is not to annoy the user. Use `info` and
+`ok` methods only for '''very''' important messages. A small
 description of the tasks you are going to perform, if they are complex
 enough.
 …
 Provide a detailed trace of any task you perform with
 <tt>verbose</tt> messages. Trace DNS queries, URLs being retrieved,
+`verbose` messages. Trace DNS queries, URLs being retrieved,
 services you have to enable or disable...
 …
 === Debugging output ===
 Don't print user-relevant messages with <tt>debug</tt>. This should be
+Don't print user-relevant messages with `debug`. This should be
 used only for developer-relevant stuff, such as tracking temporary
 contents or so.
 …
 We don't have any convention for debug levels, is it needed?
 === Use <tt>error</tt> only for fatal errors ===
 Use the <tt>error</tt> method only if this error will make the entire
+=== Use `error` only for fatal errors ===
+Use the `error` method only if this error will make the entire
 component fail. If you can handle it, or the fail is not really
 important for the component's results, use <tt>info</tt> instead. If
+important for the component's results, use `info` instead. If
 failing to download a file is OK because you can work around it, or
 you know you may have no rights to write on AFS, but that's OK, use an
 <tt>info</tt> message.
+`info` message.
 Otherwise, the component will be re-run with no need, and will keep
 …
 task we repeat over and over (f.i, do we need any especial code to do
 DNS queries or download files from the Internet?).
-[[User:Luisfmunoz|Luisfmunoz]] 00:19, 12 January 2009 (UTC)