Gecko:Interface Style Guide

Interface Design Notes

The ongoing Gecko:DeCOMtamination work means we are reformatting many interfaces. The following guidelines may be useful to help create efficient, easy-to-use interfaces.

The following hints are only meant for non-XPCOM interfaces where binary compatibility is not an issue.

General

  • Macros suck. Don't use NS_IMETHOD(Foo), use "virtual Foo ...".
  • If a certain piece of state should only be updated in stylized ways, provide methods for the stylized ways rather than exposing a Set method (e.g., see nsIFrame::AddStateBits()).
  • Don't make a method virtual until you have someone who really needs to override the method.
  • Don't pass a value as a parameter that could be obtained from one of the other parameters. For example, if your method takes an nsFoo parameter and it uses an nsBar member of nsFoo, you may be tempted to take nsBar as an additional parameter in case a caller has obtained or computed the nsBar already. Avoid doing this. It complicates the interface and may actually lead to reduced performance. Only do this if you've determined using profiling that the cost of obtaining nsBar is significant, and there is no way to make it cheaper to obtain the nsBar from nsFoo, and you can find reviewers who agree with your decision!

Parameters

  • When parameters form a logical unit, pass them as a logical unit. So, use "nsPoint aPt" instead of "nscoord x, nscoord y". Likewise use nsRect, nsMargin, nsSize, etc. Make up your own structs if you have to.
  • Declare parameters that will be modified as pointers, not references, so it is clear at the calling context which parameters may be modified.
  • Pass structs that will not be modified as const references, e.g. "const nsRect& foo". If the struct is small (say 8 bytes or less) it's OK to pass it by value. It's easy to change from const reference to value and back, anyway, because your callers don't have to change syntax.
  • Take pointers to interfaces, not references to interfaces.

Results

  • Some methods need to return an nsresult to indicate success or failure. You should try to make the method result be a real result value rather than nsresult, but you must return nsresult if the method could reasonably fail and returning 'nsnull' is not an acceptable way to indicate failure (either because the method does not return a pointer value, or the caller may want to distinguish success-returning-null from failure, or the caller may want to distinguish various causes of failure via an nsresult error code). If a method does not need to return an nsresult then it should return its logical result directly as the method return value, in which case the following rules apply. If you're deCOMtaminating an existing method then you can remove the nsresult return if either (a) the method (in every class where it's implemented) always returns NS_OK or (b) there's only one failure code, e.g., NS_ERROR_OUT_OF_MEMORY and it can be indicated with a special value for the real result, e.g. nsnull, or (c) every call site ignores the result.
  • If there are multiple results which can be thought of as forming a unit, and the method return value does not need to be an nsresult, consider declaring a struct just to hold the results and returning that (e.g., see nsIView::GetZIndex) --- if a suitable struct doesn't already exist.
  • If you return a struct that corresponds to a struct you have in some datastructure, you might be tempted to just return a const reference to the existing struct. It's probably better if you don't do this, because that datastructure struct could change and surprise the caller. Just return the struct by value, making a copy.
  • If you're returning one result as the return value and using out parameters for the rest, and one of your results is a pointer to a ref-counted interface, put that result as a raw pointer in the return value and do not add a ref. Then callers can decide whether to add a ref or not (by writing "nsIFoo* r = ..." or "nsCOMPtr<nsIFoo> r = ...".
  • If you're returning a pointer to a ref-counted interface in the return value, but for some reason it's infeasible to implement the method without adding a ref, then return an "already_AddRefed<nsIFoo>".
  • Under no circumstances return a raw pointer with a ref added, or you will be punished!
  • Any out parameters which are pointers to ref-counted interfaces should obey the normal XPCOM rules and add a ref on the returned interfaces.
  • If you're returning one result as the return value and using out parameters for the rest, and none of them are pointers to ref-counted interfaces, make the return value be the result that a caller is most likely to ignore. It's easier for the caller to ignore a return value than an out parameter.
  • A getter method that returns a single bare pointer which can never be null should lose the "Get" prefix. Thus no-parameter methods whose names are noun phrases indicate that it is safe to use their result without testing for null.

Gecko-specific Rules

  • You will almost never need to take an nsIPresContext parameter. You can usually get the prescontext by calling GetPresContext() on some frame.
  • You should never take an nsIPresShell parameter. nsIPresShell will be merged into nsIPresContext. Replace all uses of nsIPresShell with nsIPresContext.