mozilla style doc

[Alec Flett]

As a super reviewer, I've run into many of the same issues over and over 
again.. I'm starting to compile them into a larger list that I'd like 
people to start reviewing. The idea is not to teach people about 
architecture or COM or C++, but rather the conventions that we use 
throughout our code.

I'm soliciting feedback, especially in the form of a tip or two from 
fellow reviewers- something simple is fine, I'll probably know what 
you're talking about if you say "No tabs" or "leading interCaps in IDL" 
- I'll write up some nice explanations and code.

I've attached the latest version, which should be appearing here in the 
next 30 minutes:
http://www.mozilla.org/hacking/mozilla-style-guide.html

                   Alec

Title: Mozilla Coding Style Guide


  

  
    Mozilla Coding Style Guide
    
        This document attempts to explain the basic styles and patterns that are used in the mozilla codebase. New code should try to conform to these standards so that it is as easy to maintain as existing code. Of course every rule has an exception, but it's important to know the rules nonetheless!
    This is particularily directed at people new to the mozilla codebase, who are in the process of getting their code reviewed. Before getting a review, please read over this document and make sure your code conforms to the recommendations here.
    

    COM and pointers
    IDL
    Error Handling
    Strings
    Naming and Formatting code

    
    COM and pointers
    
      use nsCOMPtr<>If you don't know how to use it, start looking in the code for examples. The general rule is that the very act of typing NS_RELEASE should be a signal to you to question your code: "Should I be using nsCOMPtr here?". Generally the only valid use of NS_RELEASE are when you are storing refcounted pointers in a long-lived datastructure. 
    

    
    IDL
    
      Use leading-lowercase, or "interCaps"
      When defining a method or attribute in IDL, the first letter should be lowercase, and each following word should be capitalized. For example:
              long updateStatusBar();
    
      
    
    
      Use attributes wherever possible
    
    Whenever you are retrieving or setting a single value without any context, you should use attributes. Don't use two methods when you could use one attribute. Using attributes logically connects the getting and setting of a value, and makes scripted code look cleaner.
    This example has too many methods:
        interface nsIFoo : nsISupports {
        long getLength();
        void setLength(in long length);
        long getColor();
    };
    
    The code below will generate the exact same C++ signature, but is more script-friendly.
        interface nsIFoo : nsISupports {
        attribute long length;
        readonly attribute long color;
    };
    

    
      Use java-style constants
    

    When defining scriptable constants in IDL, the name should be all uppercase, with underscores between words:
            const long ERROR_UNDEFINED_VARIABLE = 1;
    
    
    Error handling
    
      Check for errors early and often
      Every time you make a call into an XPCOM function, you should check for an error condition. You need to do this even if you know that call will never fail fail. Why?
    
       Someone may change the callee in the future to return a failure condition.
         The object in question may live on another thread, another process, or possibly even another machine. The proxy could have failed to actually make your call in the first place.
    
  
    
      Return from errors immediately Every time an error condition happens, your knee-jerk reaction should be to return from the current function.
      Don't do this:
    
    rv = foo->Call1();
    if (NS_SUCCEEDED(rv)) {
        rv = foo->Call2();
            if (NS_SUCCEEDED(rv)) {
                rv = foo->Call3();
            }
        }
    }
    return rv;
    

    Instead, do this:

    
    rv = foo->Call1();
    if (NS_FAILED(rv)) return rv;

    rv = foo->Call2();
    if (NS_FAILED(rv)) return rv;

    rv = foo->Call3();
    if (NS_FAILED(rv)) return rv;
    

    Why? Because error handling should not obfuscate the logic of the code. The author's intent in the first example was to make 3 calls in succession, but wrapping the calls in nested if() statements obscured the most likely behavior of the code.
    

    Consider a more complicated example that actually hides a bug:

        PRBool val;
    rv = foo->GetBooleanValue(&val);
    if (NS_SUCCEEDED(rv) && val)
        foo->Call1();
    else
        foo->Call2();
    

    The intent of the author may have been that foo->Call2() would only happen when val had a false value. In fact, foo->Call2() will also be called when foo->GetBooleanValue(&val) fails. This may or may not have been the author's intent, and it is not clear from this code. Here is an updated version:

        PRBool val;
    rv = foo->GetBooleanValue(&val);
    if (NS_FAILED(rv)) return rv;
    if (val)
        foo->Call1();
    else
        foo->Call2();
    

    In this example, the author's intent is clear, and an error condition avoids both calls to foo->Call1() and foo->Call2();
    

    
    Strings
    
    
      Use the Auto form of strings for local values When declaring a local, short-lived nsString class, always use nsAutoString or nsCAutoString - these versions pre-allocate a 64-byte buffer on the stack, and avoid fragmenting the heap. Don't do this:
    
        nsresult foo() {
      nsCString bar;
      ..
    }
    
    instead:
        nsresult foo() {
      nsCAutoString bar;
      ..
    }
    

    
      Be wary of leaking values from non-XPCOM functions that return char* or PRUnichar* It is an easy trap to return an allocated string from an internal helper function, and then use that function inline in your code without freeing the value. Consider this code:
        static char *GetStringValue() {
        ..
        return resultString.ToNewCString();
    }

        ..
        WarnUser(GetStringValue());
    

    In the above example, WarnUser will get the string allocated from resultString.ToNewCString() and throw away the pointer. The resulting value is never freed. Instead, either use the string classes to make sure your string is automatically freed when it goes out of scope, or make sure that your string is freed.
    Automatic cleanup:
        static void GetStringValue(nsAWritableCString& aResult) {
        ..
        aResult.Assign("resulting string");
    }

        ..
        nsCAutoString warning;
        GetStringValue(warning);
        WarnUser(warning.get());
    
    Free the string manually:
        static char *GetStringValue() {
        ..
        return resultString.ToNewCString();
    }

        ..
        char *warning = GetStringValue();
        WarnUser(warning);
        nsCRT::Free(warning);
    

            
              Use NS_LITERAL_STRING() to avoid runtime string conversion. It is very common to need to assign the value of a literal string such as "Some String" into a unicode buffer. Instead of using nsString's AssignWithConversion and AppendWithConversion, use NS_LITERAL_STRING() instead. On most platforms, this will force the compiler to compile in a raw unicode string, and assign it directly.
    Incorrect:
        nsAutoString warning; warning.AssignWithConversion("danger will robinson!");
    ..
    foo->SetUnicodeValue(warning.get());
    
    Correct:
        nsAutoString warning(NS_LITERAL_STRING("danger will robinson!"));
    ..
    // if you'll be using the 'warning' string, you can still use it as before:
    foo->SetUnicodeValue(warning.get());

    // alternatively, use the wide string directly:
    foo->SetUnicodeValue(NS_LITERAL_STRING("danger will robinson!").get());
    
    
    Futures issues I'll be addressing:
    
       macros: NS_
       don't check after QI - use try/catch
    
    
    Naming and Formatting code

    
      No tabs
    
    
      Follow naming prefix conventions
      Variable prefixes:
    
       k=constant (e.g. kNC_child)
       g=global (e.g. gPrefService)
       m=member (e.g. mLength)
       a=argument (e.g. aCount)
    

    Global functions/macros/etc
    
       Macros begin with NS_, and are all caps (e.g. NS_IMPL_ISUPPORTS)
       Global (exported) functions begin with NS_ and use LeadingCaps (e.g. NS_NewISupportsArray)
    
    
    
    


Last modified: Thu May 10 15:26:37 -0700 2001