http://qs321.pair.com?node_id=744932

I've been asked to prepare some guidelines on coding standards and code reviews in general, across a number of languages used at work, not just Perl. After doing a bit of basic research, I've cobbled together the notes below. To gain some feedback, I'm posting an early draft here.

Note that language-specific coding standards are not covered here; these will be covered in separate coding standards documents, one for each language.

Most of the coding guidelines below were not invented by me, but derived from hopefully well-respected sources; see the Coding Standards References section below for details.

Update: see also Why Create Coding Standards and Perform Code Reviews?

General Guidelines

These are some general attributes you should strive for in your code. Remember that code maintainability is paramount.

  1. Correctness, simplicity and clarity come first. Avoid unnecessary cleverness.
  2. If you must rely on cleverness, encapsulate and comment it.
  3. Robustness, Efficiency, Maintainability.
  4. Scalability, Abstraction, Encapsulation.
  5. Uniformity in the right dimension, creativity in dimensions that matter.
  6. DRY (Don't repeat yourself). Duplication exists in data as well as code.
  7. Prefer to find errors at compile time rather than run time.
  8. Establish a rational error handling policy and follow it strictly.
  9. Throw exceptions instead of returning special values or setting flags.
  10. Know when and how to code for concurrency.
  11. Use a single-step automated build system.
  12. Practice releasing regularly.
  13. Plan to evolve the code over time.
  14. Invest in code reviews.
  15. Consider the code from the perspective of: usability, simplicity, declarativeness, expressiveness, regularity, learnability, extensibility, customizability, testability, supportability, portability, efficiency, scalability, maintainability, interoperability, robustness, type safety, thread-safety/reentrancy, exception-safety, security. Update: a few more: correctness, reliability, readability, reusability, productivity/timeliness, documentation/discoverability. Resolve any conflicts between perspectives based on requirements.
  16. Agree upon a coherent layout style and automate it.
  17. When in doubt, or when the choice is arbitrary, follow the common standard practice or idiom.
  18. Don't optimize prematurely. Benchmark before you optimize. Comment why you are optimizing. (See On Code Optimization).
  19. Don't pessimize prematurely.
  20. Don't reinvent the wheel. If there is a library method that implements the functionality you need, use it.
  21. Be const correct.
  22. Adopt a policy of zero tolerance for warnings and errors. This principally means compiling cleanly at high warning levels, but is broader than that; for example, tools such as checked STL implementations, static code analysers (e.g. Perl::Critic, FxCop), dynamic code analysers (e.g. valgrind, Purify), unit tests (e.g. Test::More, NUnit), code that emits spurious warnings/errors in customer log files, and so on. Third party files may be exempt from this policy.
  23. Use a revision control system.
  24. Write the test cases before the code. When refactoring old code (with no unit tests), write unit tests before you refactor.
  25. Add new test cases before you start debugging.
  26. Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live.

Design and Architecture

  1. Know Architectural patterns. Choose wisely. Multitier architecture is common.
  2. Coupling and Cohesion. Systems should be designed as a set of cohesive modules as loosely coupled as is reasonably feasible.
  3. Testability. Systems should be designed so that components can be easily tested in isolation.
  4. Information hiding: Minimize the exposure of implementation details; provide stable interfaces to protect the remainder of the program from the details of the implementation (which are likely to change). Don't just provide full access to the data used in the implementation. Minimize the use of global data. Avoid Action at a distance.
  5. Interfaces matter. Once an interface becomes widely used, changing it becomes practically impossible (just about anything else can be fixed in a later release).
  6. Design the module's interface first.
  7. Design interfaces that are: consistent; easy to use correctly; hard to use incorrectly; easy to read, maintain and extend; clearly documented; appropriate to your audience. Be sufficient, not complete; it is easier to add a new feature than to remove a mis-feature.
  8. As for whether and when to use OO, a simple rule of thumb is to ask "do I need more than one?": if the answer is yes, an object is indicated; if the answer is no, a module.

OO Design

  1. Favour Composition Over Inheritance.
  2. SOLID: Single-responsibility (a class should have only one responsibility), Open-closed principle (open for extension, closed for modification), Liskov substitution principle (derived classes must be substitutable for their derived classes), Interface segregation principle (clients should not be dependent on interfaces they do not use), Dependency inversion principle (program to an interface, not an implementation).
  3. GRASP: General Responsibility Assignment Software Patterns (or Principles). Guidelines for assigning responsibility to classes and objects in object-oriented design. The different patterns and principles used in GRASP are controller, creator, indirection, information expert, high cohesion, low coupling, polymorphism, protected variations, and pure fabrication.
  4. Know Software Design Patterns. Choose wisely.
  5. Dependency Injection. Don't hard-code your dependencies. Especially useful to improve testability.
  6. Define Class invariants.
  7. Immutability. Prevent inappropriate access by making your objects immutable: provide the data to their constructors, then disallow any modifications of this information thereafter. Note that Java strings are immutable. Immutability can also be useful for thread safety and in functional programming (see Pure function).

API Design Checklist

  1. Make it easy to use correctly, hard to use incorrectly. To illustrate, Scott Meyers gives a cute example of a Date class with constructor Date(int month, int day, int year), making it easy to use incorrectly (i.e. to get the parameter order wrong). Another illustration is the PBP guideline: "Use a hash of named arguments for any subroutine that has more than three parameters", which works well because humans are better at remembering names than orderings.
  2. "Play test" your API from different perspectives: newbie user, expert user, maintenance programmer, support analyst, tester. In the early stages, imagine the perfect interface without worrying about implementation constraints. Design iteratively.
  3. Names matter. Choose names that are explanatory, consistent, regular.
  4. Have guiding principles and clear requirements.
  5. Define sound conceptual models and domain abstractions.
  6. Consider whether your domain functionality is best delivered as a library (e.g. DBI), an application framework (e.g. Catalyst), or a DSL (e.g. YACC (a translator) or make (an interpreter)).
  7. Hide implementation details. Reflect the user mental model, not the implementation model. Information hiding: Minimize the exposure of implementation details; provide stable interfaces to protect the remainder of the program from the details of the implementation (which are likely to change). Don't just provide full access to the data used in the implementation.
  8. Make easy things easy, hard things possible. Huffmanize.
  9. For non-public APIs, be sufficient, not complete. It is much easier to add a new feature than to remove a mis-feature. When in doubt, leave it out.
  10. When in doubt, or when the choice is arbitrary, follow the common standard practice or idiom.
  11. Consider the API from the perspectives of: usability, simplicity, declarativeness, expressiveness, regularity, learnability, extensibility, customizability, testability, supportability, portability, efficiency, scalability, maintainability, interoperability, robustness, type safety, thread-safety/reentrancy, exception-safety, security. Resolve any conflicts between perspectives based on requirements.
  12. Error handling. Document all errors in the user's dialect. Prefer throwing exceptions to returning special values. Prefer to find errors at compile time rather than run time (e.g. PBP p.182, Named Arguments: pass as a single hash ref not a list of name/value pairs, so that some argument errors are caught at compile time).
  13. Apply the principle of least astonishment. In particular, choose sensible (expected) and secure defaults.
  14. Look for ways to eliminate ungainly parts of the API (e.g. the need for repeated boilerplate code when using the API).
  15. Plan to evolve the API over time.
  16. Learn from prior art. In particular, avoid repeating interface design mistakes of the past.

For more detail see: On Interfaces and APIs

Maintainability/Supportability

  1. Follow the de facto standard set by the code you are editing. Or change the entire source file to the new standard.
  2. Remove unused code.
  3. Assert liberally. Asserts should be used to document the constraints for a piece of code.
  4. Log liberally. Strive to log enough information to trouble-shoot a customer problem without the need to attach a debugger.
  5. The result of every file operation or API call or external command must be checked, and unexpected results handled.
  6. Any unexpected result from a file operation or API call or external command should be logged.
  7. Avoid magic numbers. Note that 0 and 1 are ok and are not considered magical.
  8. Limit and explicitly comment case "fall throughs".
  9. Avoid side effects. e.g. inside macros.

Layout

Layout rules will vary between organisations. These sort of arbitrary code layout rules should be enforced by a tool (e.g. Perl::Tidy).

  1. Use spaces not TABs.
  2. Three character indent (four is more common; get agreement and enforce with a tool).
  3. No long lines. Limit the line length to a maximum of 120 characters.
  4. No trailing whitespace on any line.
  5. Put brace on a new line.
  6. Single space around keywords, e.g. if (.
  7. Single space around binary operators, e.g. 42 + 69
  8. Single space after comment marker, e.g. "// fred" not "//fred", "# fred" not "#fred"
  9. No space around unary operators, e.g. ++i
  10. No space before parens with functions/macros, e.g. fred( 42, 69 )
  11. Single space after parens with functions/macros, e.g. fred( 42, 69 )
  12. Single space after comma with functions/macros, e.g. fred( 42, 69 )
  13. Layout lists with one item per line; this makes it easier to see changes in version control.
  14. One declaration per line.
  15. Function calls with more than two arguments should have the arguments aligned vertically.
  16. Avoid big-arse functions and methods. Ditto for large classes and large files.
  17. Avoid deep nesting.
  18. Always use braces with if statements, while loops, etc. This makes changes shorter and clearer in version control.

Naming

  1. Use descriptive, explanatory, consistent and regular names.
  2. Favour readability over brevity.
  3. Avoid identifiers that conflict with keywords.
  4. Short names for short scopes, longer names for longer scopes.
  5. Avoid ambiguous words in names (e.g. Don't abbreviate "Number" to "No"; "Num" is a better choice).
  6. For packages and classes a good naming scheme is: Abstract_noun, Abstract_noun::Adjective, Abstract_noun::Adjective1::Adjective2. For example: Disk; Disk::Audio; Disk::DVD; Disk::DVD::Rewritable.

Encapsulation

  1. Avoid non-const global variables.
  2. Minimize the scope of variables, pragmas, etc..
  3. Minimize the visibility of variables.
  4. Don't overload variables with multiple meanings.

Dependencies

When should you depend on another CPAN module rather than write your own code?

  1. Don't introduce dependencies lightly. Every module you add as a dependency is a module that can restrict your module -- if one of your module's dependencies is Linux-only, for example, then your module is now Linux-only; if another requires Perl 5.20+ so do you; if one of your dependencies has a bug, you also have that bug; if a new release of one of your dependencies fails, the likelihood of your release being unable to install increases; take care with dependencies having a different license to yours.
  2. Don't add developer convenience modules as a dependency. (As noted at Release::Checklist, there are two types of modules: functional modules, like DBI, and developer convenience modules, like Modern::Perl).
  3. It's usually best to use popular, quality CPAN modules in complex domains (e.g. DBI and XML) rather than roll your own. Doing so allows you to leverage the work of experts in fields that you are probably not expert in. Moreover, widely used CPAN modules tend to be robust and have fewer bugs than code you write yourself because they are tested by more users and in many different environments.
  4. For small and simple modules, on the other hand, such as slurping a file, you may prefer to roll your own code rather than pay the dependency cost of an external module.
  5. Cost vs Risk. Though using CPAN modules seems "free", there are hidden snags. What if your dependent module has a security vulnerability? What if the author abandons it? How quickly can you isolate/troubleshoot a bug in its code?
  6. Quality and Trust. Before introducing a dependency, it's worth checking CPAN ratings, Kwalitee score, bug counts, how quickly are bugs fixed etc. Does it contain gratuitous/unnecessary dependencies? (the ::Tiny CPAN modules were a reaction against modules that seemed to haul in half of CPAN as dependencies).
  7. Popularity. When you use a 3rd party module, you want it to be popular and widely supported; you want to be able to ask for advice on using it; you don't want it to die. Moreover, if your module depends on a very popular CPAN module, there's a good chance your module's users will already have it installed.

Comments and Documentation

  1. Prefer to make the code obvious.
  2. Don't belabour the obvious.
  3. Generally, comments should describe what and why, not how.
  4. Remove commented-out code, unless it helps to understand the code, then clearly mark it as such.
  5. Update comments when you change code.
  6. Include a comment block on every non-trivial method describing its purpose; each parameter should be documented as: input, output, or "inout" ("inout" parameters should be used sparingly).
  7. Major components should have a larger comment block describing their purpose, rationale, etc.
  8. There should be a comment on any code that is likely to look wrong or confusing to the next person reading the code.
  9. Every non-local named entity (function, variable, class, macro, struct, ...) should be commented.
  10. Separate user versus maintainer documentation.
  11. CPAN Module Documentation. Tutorial and Reference; Examples and Cookbook; Maintainer; How your module is different to similar ones; Change log; Notes re portability, configuration & environment, performance, dependencies, bugs, limits, caveats, diagnostics, bug reporting.

Portability

  1. Assume file names are case insensitive in that you cannot, in general, have two different files called fred and Fred.
  2. Source code file names should be all lower case.
  3. File names should only contain A-Z, a-z, 0-9, ".", "_", "-".
  4. Strive to structure code around "capabilities" rather than specific platforms. For example, "if HAVE_SHADOW_PASSWORDS" rather than "if SOLARIS_8". And define the capabilities in one place only (e.g. config.h).
  5. Organise the code so that machine dependent and machine independent code reside in separate files.
  6. Abstract hardware and external interfaces in a module and have all other code call that module and not call the hardware/external interface directly.
  7. As far as possible, write code to a widely supported portable standard (e.g. ANSI C, POSIX, ...) and only use machine specific facilities when absolutely necessary.
  8. Recognize and avoid non-portable constructs. For example, strive to avoid relying on ASCII character set, big v little-endian, 32-bit v 64-bit ints, pointer to int conversion, sign extension, and so on.

User Interfaces

  1. All command line tools should provide a usage option to explain the usage of the command. Always include at least one example in the usage description. You must at least support the -h option for help and optionally may support --help.
  2. Provide appropriate, clear feedback to the user of the progress of any long running operations and make them easy to cancel and safely rerunnable.

GUI User Interfaces

  1. Have clear objectives and guiding principles.
  2. Define personas; design to satisfy their goals.
  3. Adopt the user's perspective. Involve users in design. Perform usability tests. Give the user control. Make it configurable. Design iteratively.
  4. GUIs should reflect the user mental model, not the implementation model. Hide implementation details.
  5. Communicate actions to user. Provide feedback. Anticipate errors. Forgive errors. Offer warnings.
  6. Cater for both novice and expert. For novice: easy-to-learn, discoverable, tips, help. For expert: efficiency, flexibility, shortcuts, customizability. Optimize for intermediates.
  7. Keep interfaces simple, natural, consistent, attractive. Try to limit to seven simultaneous concepts.
  8. Use real-world metaphors.
  9. Ask forgiveness, not permission. Make all actions reversible.
  10. Eliminate excise.
  11. Be polite; remember what the user entered last time.
  12. Avoid dialog boxes as much as possible; don't use them to report normalcy.
  13. Provide "wizards" for complex procedural tasks.

Security

  1. Define security requirements as part of product requirements.
  2. Conduct security reviews (creating a threat model) if warranted by requirements.
  3. Know where to look for exploit notices and stay up-to-date.
  4. Use least privilege; only run with superuser privilege when you need to.
  5. When using fixed length buffers, ensure that any possible overflow is handled.
  6. Handle all errors (e.g. don't ignore error returns). Fail securely.
  7. Define secure defaults.
  8. Know how to call external components safely.
  9. Know how to handle insecure environment (e.g. environment variables, umask, inherited file descriptors, symbolic links, temporary files, child processes, ...).
  10. Validate insecure external data (e.g. input to program, parameters to an exported API).
  11. Beware of race conditions.
  12. Avoid canonical file paths and URLs.
  13. Use a security code review checklist.
  14. Use security code analysis tools.
  15. Minimize your attack surface.
  16. Know how to defend against common known attacks.
  17. Defend in depth.
  18. Don't tell the attacker anything.
  19. Don't mix code and data.
  20. Don't depend on security through obscurity.
  21. Heed compiler warnings.
  22. Architect and design for security policies. Keep it as simple as possible.
  23. Default deny. Base access decisions on permission not exclusion, by default deny access.
  24. Use effective QA: fuzz testing, penetration testing, source code audits.
  25. Adopt a secure coding standard.

See also: Re: Security techniques every programmer should know (Security References)

Internationalization (i18n) and Localization (L10n)

These domains warrant their own section. However, both these domains can be incredibly challenging. :-) For now, this is just a stub section.

  1. Define the product internationalization and localization goals.

Some Famous Programming Quotes

Code Reviews

The two principal types of code review are: formal and lightweight.

Formal code reviews require significant planning, training and resources; they are carried out in multiple phases by multiple participants playing various roles. The most well-known formal code review method is the Fagan inspection.

Lightweight code reviews, having less overhead than formal ones, are cheaper to conduct. The Best Kept Secrets of Peer Code Review book argues that lightweight code reviews are cheaper to perform than formal ones and can be just as effective. The four most common types of lightweight code review are:

  1. Over-the-shoulder
  2. Email pass-around
  3. Pair programming
  4. Tool-assisted

Lightweight Code Review Tips

Cited in Best Kept Secrets of Peer Code Review, the conclusions drawn from a lightweight code review case study at Cisco are:

  1. LOC under review should be less than 200 and not exceed 400. Larger LOCs tend to overwhelm reviewers.
  2. Total review time should be less than 60 minutes and not exceed 90. Defect detection rates plummet after 90 minutes.
  3. Inspection rates less than 300 LOC/hour result in best defect detection. Expect to miss a significant percentage of defects if faster than 500 LOC/hour.
  4. Expect defect rates of around 15 per hour.
  5. Authors who prepare for the review with annotations and explanations have far fewer defects than those that do not.

The Seven Deadly Sins of Software Reviews

According to Karl Wiegers, the Seven Deadly Sins of Software Reviews are:

  1. Participants don't understand the review process.
  2. Reviewers critique the producer, not the product.
  3. Reviews are not planned.
  4. Review meetings drift into problem-solving.
  5. Reviewers are not prepared.
  6. The wrong people participate.
  7. Reviewers focus on style, not substance.

Miscellaneous Code Review Tips

  1. Before the code review, push the code review through a tool that checks for straightforward layout and stylistic issues; this avoids wasting time on trivia during the review.
  2. Most of the code review work should be done before the code review meeting.
  3. The code review should be in writing.
  4. Have at least two code reviewers.

Tool-assisted Code Review

Guido van Rossum's first project at Google was Mondrian, a code review tool. Though he was unable to open source that work, he has since released the open source Rietveld code review tool. An exhaustive list of code review tools can be found at Survey of Code Review Tools.

In addition to tools that streamline the administrative side of the code review process, source code analysis tools can further be useful during code reviews. These tools fall into three broad categories:

  1. Code Formatters. e.g. Perl::Tidy.
  2. Static Code Analysers. e.g. Perl::Critic.
  3. Dynamic Code Analysers. e.g. valgrind.

Coding Standards References

Code Review References

References Added Later

More References Added Later

Dealing with duplicated code:

Updated 20 Feb: Added new "Design" and "Portability" sections. Updated 3-mar: Added new "Security" and "Internationalization and Localization" sections; added more General Guidelines; added more references. Updated Nov/Dec 2017: Added "OO Design" and "References Added Later" sections; extended "Comments" section to "Comments & Documentation". Aug 2020: added naming scheme for packages and classes. Added new Dependencies section from Writing Solid CPAN Modules. Mar 2021: Moved "Correctness, simplicity and clarity" to first bullet point (see salva's response below). Mar 2021: added "API Design Checklist" section, adapted from On Interfaces and APIs. June 2021: Comments and documentation section: added input, output, or "inout" when documenting function parameters (see Re^2: flower box comments).

Replies are listed 'Best First'.
Re: On Coding Standards and Code Reviews
by ELISHEVA (Prior) on Feb 19, 2009 at 09:41 UTC
    My first impression is that "whew! you've put a lot of work into this." My second impression is that your work has just begun.

    The list is too long and the potential conflicts between some of the various standards you've cited need to be explored. So too, the match between the list and the problem areas for your particular team. The key is focus: too much information is equivalent to no information at all.

    Conflicts between "best practices" isn't necessarily bad: good architecture is often a delicate balance between competing principles (e.g. abstraction and simplicity). In fact, discussing and exploring competing principles as a team is a great way to develop good programming judgment about which principles should apply when.

    I also noticed that nowhere in your list of sources did you mention interviews with your team members. There is another thought you might want to consider that came up in a recent discussion between tilly and myself: mentorship. Programming is as much an art as a science and if there are experienced members of your team with proven design and implementation skills, it is a good idea to ask them for their insights and to encourage them to coach the less (software wise) mature members of the team. Sometimes the bluebird of happiness is flying around in your own backyard. A design principle taught and tied to the experience of an old hand and a member of the team is going to carry a lot more weight than the rhetorical pronouncements of the latest gurus.

    Best of luck with this project, beth

    P.S. If you have a manager that doesn't get that last bit (rhetoric vs. ...) I have a nice article from Harvard Business Review on the Rhetoric of Action that I can dig up for you - and I'm sure I can find others as well. Just msg me.

Re: On Coding Standards and Code Reviews
by Tanktalus (Canon) on Feb 19, 2009 at 05:57 UTC

    I guess it may be just me, but I'm curious as to why. Some of your suggestions/commandments are purely arbitrary, and I don't use that in a negative light as sometimes there is no best choice and consistency is more important, but I'm still curious as to why you chose that arbitrary rule over another similarly-weighted controversial rule.

    Just to pick on the first one that popped out, using spaces over tabs. Don't get me wrong, I agree with it, but I know it's contentious, and would prefer to see something that expanded on why. This goes doubly, if not more, if you're going to need to not just defend your rules against peers in your organisation, but against every single new hire for the rest of your tenure there. It also allows the rules to be properly challenged if, for example, the significance of your decision is reduced.

    As an example, continuing to pick on the spaces vs tabs, perhaps it is desirable for each developer to use the tools they're most comfortable with. That may mean some of your developers using eclipse, others using emacs, others using vi, and the odd wacko using notepad, as long as each is highly productive. But then, perhaps your management (either personnel or technical) decides for some reason to standardise on an environment - whether that's MS Visual Studio, Eclipse, or Komodo. Now maybe the reason for spaces goes away, and it can be revisited. But without a continued, on-going understanding of the reasoning, your rules become unchallengeable shackles or cargo cult, possibly long outliving their usefulness.

    I know you have lots of links, but, without going through each one manually, it's difficult to know (in general) which link defends which rule. Some are obvious, but not all, so you should still have as direct of links as possible for most items.

    My second suggestion would be to store the whole thing in a corporate wiki where others can add their reasons for (or possibly even against) each dictum, making it (hopefully) feel less like dictum, and more like group selection, and thus easier to self-enforce. It also would help keep the document living which helps it stay in use, as well as keeps it available for challenge.

      Just to pick on the first one that popped out, using spaces over tabs. Don't get me wrong, I agree with it, but I know it's contentious, and would prefer to see something that expanded on why.
      This is discussed in Perl Best Practices, page 20, "Indent with spaces, not tabs". Quoting Conway:
      Tabs are a bad choice for indenting code, even if you set your editor's tabspacing to four columns. Tabs do not appear the same when printed on different output devices, or pasted into a word-processor document, or even just viewed in someone else's differently tabspaced editor.
      Not being an "expert book author", many folks here won't listen to me, so I prefer to point them to a well-respected reference, such as PBP.

      For some of these layout rules (e.g. brace placement), we already fought a war some years ago and I don't want to reopen old wounds. With over ten million lines of existing code, it doesn't seem worthwhile to change longstanding company code layout conventions.

      I know you have lots of links, but, without going through each one manually, it's difficult to know (in general) which link defends which rule. Some are obvious, but not all, so you should still have as direct of links as possible for most items.
      Agreed. I'll try to provide a reference/s for each rule. I expect that will prove to be important for some folks here who may object to blindly accepting a rule without a supporting reference.

        Tabs are a bad choice for indenting code, even if you set your editor's tabspacing to four columns. Tabs do not appear the same when printed on different output devices, or pasted into a word-processor document, or even just viewed in someone else's differently tabspaced editor.

        Having lot of respect for Damian Conway I would like to point out that almost all useful editors allow you to define the tab length. Be it 2, 4 or 8 spaces. Using tabs allows every developer to have their own indentation level by defining the tab width in their preferred editor/pager/viewer settings. Using spaces forces one indentation scheme (and forcing people is a bad thing). I always tell people who are arguing over indentation that whether you choose to use tabs or spaces you must use it systematically. Do not mix tabs with spaces.

        I use spaces my self (spaces are what I am accustomed to), but I do not despise tabs.

        --
        seek $her, $from, $everywhere if exists $true{love};

        For Pete's sake people it's tabs and spaces!

        How many hours of productive time has been wasted in meetings arguing the pros and cons of each? Does Perl care whether you've used spaces over tabs or tabs over spaces? We've already agreed that most editors can seamlessly convert between tabs and spaces of any desired indent size. The pettiness of it just blows my mind.

        Here's a standard: forget standards. Use Perltidy.

        A large portion (if not all) of the coding standards listed here could be configured into Perltidy. If every piece of code passed/used/saved anywhere in the company is enforced through Perltidy, then all code will conform to a single layout standard. The coders don't need standards, they can code to what ever style they feel comfortable with. After a pass through Perltidy everybody's code will conform to the same single standard.

        In time coders will make an effort to conform to that same standard that they've been immersed in because it's just easier to adjust one's personal style choices than to buck miles of existing code. But even if they don't come around, who cares? In the end the code will come out formatted to standards regardless, and all the time that would have been wasted arguing over standards can be put to better use getting actual work done.

        end grump
Re: On Coding Standards and Code Reviews
by dHarry (Abbot) on Feb 19, 2009 at 10:09 UTC

    I thank you for the effort and up-voted you accordingly! I do have some remarks though.

    Most of the general guidelines are too general to be of any use. How are you going to address them? What do you need to do when, and how are you going verify they have been met?

    Take for example Robustness, how to address that?

    What I miss is the Software Development Life Cycle, the process. You need to make the abstract statements tangible and embed them in a process. In my experience this is the only way to make it work, especially when projects get bigger and/or the SW has a zero fault tolerance architecture. i.e. SW for a nuclear plant, medical applications etc.

    Also where does architecture comes in? What I mean is the general guidelines may conflict with each other. Then you will have to determine what is more important, likely on a per project basis. I like to view things like Robustness, Efficiency, Maintainability, Scalability, etc as “Quality Attributes” in the architecture and give them weights depending on their importance (depending on the specific project). With a simple excel sheet you can make a more founded decision on what to focus on. There are always restrictions (budget, time,...) and chances are you can’t do all things in your list. Focusing on the important ones and doing them good makes more sense then doing all of them but half.

    I've been asked to prepare some guidelines on coding standards and code reviews

    Coding Standards and Code reviews are typically instruments of the (much) wider area of Software Quality Assurance. The code reviews they fall in the SQA activity category SW verification/validation.

    I don’t know the size of your company but I assume it must have at least one QA guy, maybe even a full-blown QA department. In any case QA people need to be involved in these type of activities.

    HTH

      Thanks for the feedback. This is the sort of feedback I was hoping for, so I can be prepared before I meet with our QA guy. :-) To answer your question, we have one QA guy and I'll need to refine these notes before I meet with him. As you might expect, he's more interested in process than I am, his primary concerns being things like "how should we enforce the coding standard?". In the past, we've sorta had coding standards (actually, too many of them) and informal code reviews, but they've never been enforced consistently across the organisation, especially when deadlines are tight.

        One way to enforce code quality and integrity in the process is to choose a process model that can facilitate it. Agile methods are good at it. Especially I would recommend looking at Scrum and pay attention to the Definition of Done.

        A self guiding process is the best tool to get teams happy, efficient and committed. This includes code quality and standards. I would not dictate them except on a very high level (setting some thresholds). IMO the tools do not matter that much, the result is what I would be interested in. And a good process that includes tools for correcting bad ways of working is priceless.

        --
        seek $her, $from, $everywhere if exists $true{love};
Re: On Coding Standards and Code Reviews
by gwadej (Chaplain) on Feb 19, 2009 at 15:00 UTC

    Like many others, I'm very impressed with what you've written. I see it as a survey of the things we might possibly want to consider when doing reviews and such.

    I recently worked in an organization which had a requirement of tool-assisted review for every piece of code that went into version control. I can say for certain that a much smaller list would be more useful for focusing reviews.

    Picking a single coding standard that everyone can work with is a very good idea. Separate from that a (short) checklist of things to be aware of when reviewing is helpful.

    Were I beginning this kind of a process (again), I'd probably start with your list and start paring them down. Anything that everyone does already (or strongly agrees with) does not really need to be on the checklist. Anything that everyone agrees with and tries to do should be on the list. Start with the low-hanging fruit. What gives the most advantage with the least disagreement?

    In my experience, the bigger the list the more likely to generate one of two reactions:

    • "That's nice." and everyone ignores it
    • "I disagree with this one little nit, so I won't do it."

    Generating a shorter checklist from your overall list is less likely to trigger either of these reactions.

    The great thing about your list, for me at least, is that it gives a broad overview from which someone could pick what is important for their group.

    G. Wade
Re: On Coding Standards and Code Reviews
by CountZero (Bishop) on Feb 19, 2009 at 17:04 UTC
    Wow, impressive compilation of standards! ++ already just for that.

    My view on standards, whether for programming or any other team-work related matter, is that you must have some clear and well-understood principles which will be enforced in any case. If it has exceptions it should not go in the standards. The standard may be controversial, but once someone (with the power to do so) decided it is the standard, there is no discussion and the team follows it or you get out of the team.

    Of course that means that the team members must be able to follow the standards and it is my experience, if you have more than 5 principles/rules/standards to follow, you are too likely to miss one and people will understand/accept that you missed one. That is a slippery slope and before you know it everyone will do again as they did before and not follow any of the rules.

    So pick five (three is even better) practical and concrete principles, rules, standards, best practices, ... post them in a conspicuous place and rigidly enforce them.

    Things like "correctness comes first" is not a good principle: if it is not correct it is wrong and it simply doesn't count. This is a meta-principle or pre-condition or part of the environment and comes before you even consider your rules.

    "Robustness, Efficiency, Maintainability" is too vague and cannot be enforced.

    "Benchmark before you optimize", "Log all errors" and "The result of every file operation or API call must be checked, and unexpected results handled." on the other hand are examples of good standards and are easy to maintain and enforce.

    If you can settle on five general rules, maybe you can add three for each of the different languages.

    Lay-out should be entrusted to a code-prettifyer such as Perl-Tidy or similar, rather than put this in a rule-set. Then everybody can code as they please and before they check in their code it gets transformed into whatever form or layout is required. Some code-repositories can even do that automatically. If you cannot entrust it to a code-prettifyer, forget it. Do you really want to check the lay-out of someone elses code for the rest of your career?

    CountZero

    A program should be light and agile, its subroutines connected like a string of pearls. The spirit and intent of the program should be retained throughout. There should be neither too little or too much, neither needless loops nor useless variables, neither lack of structure nor overwhelming rigidity." - The Tao of Programming, 4.1 - Geoffrey James

Re: On Coding Standards and Code Reviews
by salva (Canon) on Feb 19, 2009 at 08:46 UTC
    1. Robustness, Efficiency, Maintainability.

    Wrong! for most applications efficiency* should not be a priority anymore... at least not your second top priority.

    And BTW, correctness is the first one!

    (* efficiency understood as getting your application to use as few resources as possible, not as programmer efficiency or any other thing)

      And BTW, correctness is the first one!

      You missed the BEGIN at point 4 ;-)

      4. Correctness, simplicity and clarity come first. Don't be clever.

      (Emphasis mine)

        I think this brings up another good point. Put your rules in order. Anyone reading them will find the first rule that seems to apply and stop reading. If something is most important, put it first. There will always be conflicts (speed of app vs speed of development), and it is important to put them in the order which you want them to supercede each other. This is not supposed to be a mystery novel, building suspense. Even then, there will sometimes still be times where you can't follow the rules - what then? Obviously, a senior developer (or just a prick like me) will ignore rules that don't apply in a specific circumstance (perhaps you have something running during mouse movement - it obviously has to be fast, or the whole system will die, even at the expense of readability of code). Perhaps a guideline on how to determine you're in a special case, but, more importantly, what to do with that special case. Perhaps a comment block to highlight the non-standard code, why, and what it really does in plain English rather than the mess of code it may really be?

        (++ for both of you for noticing it and bringing it up :-) )

Re: On Coding Standards and Code Reviews
by JavaFan (Canon) on Feb 19, 2009 at 23:05 UTC
    No long lines. Limit the line length to a maximum of 120 characters.
    In my book, 120 characters is a very long line. When editing code I didn't write, I always leave the formatting as is. It usually isn't my writing style, but I can write Perl even if it doesn't have the spaces where I usually put them. I'm not at all a fan of enforcing formatting - I find that as counter productive as enforcing wearing ties or high heels.

    But there's one formatting thing I change when editing code. And that's breaking lines exceeding 80 characters. I've my own set of stylistic issues (which I don't enforce, or want to enforce on anyway - they are just for myself), and each point will be broken once every blue moon for some reason. Except the 80 character line limit. That never, ever gets broken. 80 characters is the absolute limit a line may have.

Re: On Coding Standards and Code Reviews
by talexb (Chancellor) on Feb 19, 2009 at 21:50 UTC

    I like my code laid out in a particular way, but your section on Code Layout really clamps down on a particular style. I'm not sure if it's going to be worth the effort that make absolutely sure that everyone's following exactly the same format.

    A better approach is somewhere between 'I like to format my code like this' and 'I'm making changes to some existing code so I'm going to modify my coding style so the new code doesn't clash too much with the old code'

    For me, a code review is more about 'is it readable and maintainable?' than 'is the one correct true style being followed?'

    Alex / talexb / Toronto

    "Groklaw is the open-source mentality applied to legal research" ~ Linus Torvalds

Re: On Coding Standards and Code Reviews
by mr_mischief (Monsignor) on Feb 19, 2009 at 19:01 UTC
    Some of your rules seem repetitive. Perhaps you should move the ones you feel are worth repeating toward the top and have fewer rules overall. A policy has the most effect when it is actually followed. It's more easily followed if it's more easily digested and even memorized. Fewer core rules with more moved into the references and subrules sections might make it easier to follow.
Re: On Coding Standards and Code Reviews
by wol (Hermit) on Feb 19, 2009 at 18:08 UTC
    Remember that code maintainability is paramount.

    Here here. By definition, if the code is maintainable, you can change it to also conform to any/all of the other standards afterwards.

    Of course, there's plenty to say about how to make code maintainable...

    --
    use JAPH;
    print JAPH::asString();

Re: On Coding Standards and Code Reviews
by Anonymous Monk on Mar 02, 2009 at 20:35 UTC
    I would probably add writing unit tests for the code you have written or going to refactor (if there are no unit tests written earlier).

      Thanks. I agree strongly with this, just didn't think of it at the time. So I've added this tip to the end of the "General Guidelines" section. I also added a few more related ones from Ten Essential Development Practices by Damian Conway.

Re: On Coding Standards and Code Reviews
by ack (Deacon) on Mar 02, 2009 at 18:42 UTC

    Wow! Great node. Lots of ideas and comments; all great!

    This node happens to hit home for me right now. I've been given the task of developing and intitutionalizing a set of System Engineering "Best Practices" for our work developing and fielding a variety of systems and I am facing many, many of the exact same issues as brought up here...though mine pertain to overall engineering rather than coding.

    I have found many of the ideas and comments on this node surprisingly meaningful and applicable to my challenge and will weight all the comments carefully to find a multitude of nuggets.

    Some ever spur me to think about adding some coding-specific "best practices" for our systems' software, too.

    PM always delights, astounds, and teaches so much to so many. Thanks to you all!

    ack Albuquerque, NM