blob: 957ab0295f160211188a85ff94d99fbfaee67d29 [file] [log] [blame]
Dan Morrilla9788cd2009-11-15 11:49:30 -08001page.title=Code Style Guidelines for Contributors
Dan Morrill3cd199f2009-11-06 14:04:16 -08002doc.type=source
3@jd:body
Dan Morrill55de6812009-11-15 15:46:36 -08004<p>The rules below are not guidelines or recommendations, but strict rules.
5Contributions to Android generally <b>will not be accepted if they do not
6adhere to these rules.</b>
Dan Morrill3cd199f2009-11-06 14:04:16 -08007</p>
Dan Morrill55de6812009-11-15 15:46:36 -08008<p>Not all existing code follows these rules, but all new code is expected to.</p>
Dan Morrill3cd199f2009-11-06 14:04:16 -08009<h1><a>Java Language Rules</a>
10</h1>
11<p>We follow standard Java coding conventions. We add a few rules:
12</p>
Dan Morrill1aad93f2010-05-28 14:49:47 -070013<ol><li><a href="#exceptionsIgnore">Exceptions</a>: Never catch and ignore them without explanation.</li>
14<li><a href="#exceptionsAll">Exceptions</a>: do not catch generic Exception, except in library code at the root of the stack.</li>
15<li><a href="#finalizers">Finalizers</a>: generally don't use them.</li>
16<li><a href="#imports">Imports</a>: Fully qualify imports</li>
Dan Morrill3cd199f2009-11-06 14:04:16 -080017</ol>
Dan Morrill1aad93f2010-05-28 14:49:47 -070018<h1>Java Library Rules</h1>
19<p>There are conventions for using Android's Java libraries and tools. In some
20cases, the convention has changed in important ways and older code might use a
21deprecated pattern or library. When working with such code, it's okay to
22continue the existing style (see <a href="#consistency">Consistency</a>). When
23creating new components never use deprecated libraries.</p>
24<h1>Java Style Rules</h1>
25<p>Programs are much easier to maintain when all files have a consistent
26style. We follow the standard Java coding style, as defined by Sun in their <a
27href="http://java.sun.com/docs/codeconv/html/CodeConvTOC.doc.html">Code
28Conventions for the Java Programming Language</a>, with a few exceptions and
29additions. This style guide is comprehensive and detailed and is in common
30usage in the Java community.</p>
31<p>In addition, we enforce the following style rules:</p>
32<ol><li><a href="#javadoc">Comments/Javadoc</a>: write it; use standard style</li>
33<li><a href="#shortmethods">Short methods</a>: don't write giant methods</li>
34<li>Fields: should either be at the top of the file, or immediately before the methods that use them</li>
35<li><a href="#localvariables">Local variables</a>: limit the scope</li>
36<li><a href="#import_style">Imports</a>: android; third party alphabetical; java(x)</li>
37<li><a href="#indentation">Indentation</a>: 4 spaces, no tabs.</li>
38<li><a href="#linelen">Line length</a>: 100 columns</li>
39<li><a href="#field_names">Field names</a>: Non-public, non-static fields start with m.</li>
40<li><a href="#braces">Braces</a>: Opening braces don't go on their own line.</li>
41<li><a href="#annotations">Annotations</a>: Use the standard annotations.</li>
42<li><a href="#acronyms">Acronyms are words</a>: Treat acronyms as words in names, yielding XmlHttpRequest, getUrl(), etc.</li>
43<li><a href="#todo">TODO style</a>: "TODO: write this description"</li>
44<li><a href="#consistency">Consistency</a>: Look at what's around you!</li>
45<li><a href="#logging">Logging</a>: Be careful with logging. It's expensive.</li>
Dan Morrill3cd199f2009-11-06 14:04:16 -080046</ol>
Dan Morrill1aad93f2010-05-28 14:49:47 -070047<h1>Javatests Style Rules</h1>
48<ol>
49<li><a href="#testmethodnames">Naming test methods</a>: testMethod_specificCase is ok</li>
Dan Morrill3cd199f2009-11-06 14:04:16 -080050</ol>
Dan Morrill1aad93f2010-05-28 14:49:47 -070051<h2>Java Language Rules</h2>
52<h2><a name="exceptionsIgnore"></a>Exceptions: do not ignore</h2>
53<p>Sometimes it is tempting to write code that completely ignores an exception
54like this:</p>
55<pre>void setServerPort(String value) {
56 try {
57 serverPort = Integer.parseInt(value);
58 } catch (NumberFormatException e) { }
59}</pre>
60<p>You must never do this. While you may think that your code will never
61encounter this error condition or that it is not important to handle it,
62ignoring exceptions like above creates mines in your code for someone else to
63trip over some day. You must handle every Exception in your code in some
64principled way. The specific handling varies depending on the case.</p>
65<blockquote>Anytime somebody has an empty catch clause they should have a
66creepy feeling. There are definitely times when it is actually the correct
67thing to do, but at least you have to think about it. In Java you can't escape
68the creepy feeling.
69-<a href="http://www.artima.com/intv/solid4.html">James
70Gosling</a></blockquote>
71<p>Acceptable alternatives (in order of preference) are:</p>
72<ul>
73<li>Throw the exception up to the caller of your method.
74<pre>void setServerPort(String value) throws NumberFormatException {
75 serverPort = Integer.parseInt(value);
76}</pre></li>
Dan Morrill3cd199f2009-11-06 14:04:16 -080077<li>Throw a new exception that's appropriate to your level of abstraction.
Dan Morrill1aad93f2010-05-28 14:49:47 -070078<pre>void setServerPort(String value) throws ConfigurationException {
79 try {
80 serverPort = Integer.parseInt(value);
81 } catch (NumberFormatException e) {
82 throw new ConfigurationException("Port " + value + " is not valid.");
83 }
84}</pre></li>
85<li>Handle the error gracefully and substitute an appropriate value in the
86catch {} block.
87<pre>/** Set port. If value is not a valid number, 80 is substituted. */
88void setServerPort(String value) {
89 try {
90 serverPort = Integer.parseInt(value);
91 } catch (NumberFormatException e) {
92 serverPort = 80; // default port for server
93 }
94}</pre></li>
95<li>Catch the Exception and throw a new RuntimeException. This is dangerous:
96only do it if you are positive that if this error occurs, the appropriate
97thing to do is crash.
98<pre>/** Set port. If value is not a valid number, die. */
99void setServerPort(String value) {
100 try {
101 serverPort = Integer.parseInt(value);
102 } catch (NumberFormatException e) {
103 throw new RuntimeException("port " + value " is invalid, ", e);
104 }
105}</pre>
106Note that the original exception is passed to the constructor for
107RuntimeException. If your code must compile under Java 1.3, you will need to
108omit the exception that is the cause.</li>
109<li>Last resort: if you are confident that actually ignoring the exception is
110appropriate then you may ignore it, but you must also comment why with a good
111reason:
112<pre>/** If value is not a valid number, original port number is used. */
113void setServerPort(String value) {
114 try {
115 serverPort = Integer.parseInt(value);
116 } catch (NumberFormatException e) {
117 // Method is documented to just ignore invalid user input.
118 // serverPort will just be unchanged.
119 }
120}</pre></li>
Dan Morrill3cd199f2009-11-06 14:04:16 -0800121</ul>
Dan Morrill1aad93f2010-05-28 14:49:47 -0700122<h2><a name="exceptionsAll"></a>Exceptions: do not catch generic Exception</h2>
123<p>Sometimes it is tempting to be lazy when catching exceptions and do
124something like this:</p>
125<pre>try {
126 someComplicatedIOFunction(); // may throw IOException
127 someComplicatedParsingFunction(); // may throw ParsingException
128 someComplicatedSecurityFunction(); // may throw SecurityException
129 // phew, made it all the way
130} catch (Exception e) { // I'll just catch all exceptions
131 handleError(); // with one generic handler!
132}</pre>
133<p>You should not do this. In almost all cases it is inappropriate to catch
134generic Exception or Throwable, preferably not Throwable, because it includes
135Error exceptions as well. It is very dangerous. It means that Exceptions you
136never expected (including RuntimeExceptions like ClassCastException) end up
137getting caught in application-level error handling. It obscures the failure
138handling properties of your code. It means if someone adds a new type of
139Exception in the code you're calling, the compiler won't help you realize you
140need to handle that error differently. And in most cases you shouldn't be
141handling different types of exception the same way, anyway.</p>
142<p>There are rare exceptions to this rule: certain test code and top-level
143code where you want to catch all kinds of errors (to prevent them from showing
144up in a UI, or to keep a batch job running). In that case you may catch
145generic Exception (or Throwable) and handle the error appropriately. You
146should think very carefully before doing this, though, and put in comments
147explaining why it is safe in this place.</p>
148<p>Alternatives to catching generic Exception:</p>
149<ul>
150<li>Catch each exception separately as separate catch blocks after a single
151try. This can be awkward but is still preferable to catching all Exceptions.
152Beware repeating too much code in the catch blocks.</li>
153<li>Refactor your code to have more fine-grained error handling, with multiple
154try blocks. Split up the IO from the parsing, handle errors separately in each
155case.</li>
156<li>Rethrow the exception. Many times you don't need to catch the exception at
157this level anyway, just let the method throw it.</li>
Dan Morrill3cd199f2009-11-06 14:04:16 -0800158</ul>
Dan Morrill1aad93f2010-05-28 14:49:47 -0700159<p>Remember: exceptions are your friend! When the compiler complains you're
160not catching an exception, don't scowl. Smile: the compiler just made it
161easier for you to catch runtime problems in your code.</p>
162<h2><a name="finalizers"></a>Finalizers</h2>
163<p><b>What it is</b>: Finalizers are a way to have a chunk of code executed
164when an object is garbage collected.</p>
165<p><b>Pros</b>: can be handy for doing cleanup, particularly of external
166resources.</p>
167<p><b>Cons</b>: there are no guarantees as to when a finalizer will be called,
168or even that it will be called at all.</p>
169<p><b>Decision</b>: we don't use finalizers. In most cases, you can do what
170you need from a finalizer with good exception handling. If you absolutely need
171it, define a close() method (or the like) and document exactly when that
172method needs to be called. See InputStream for an example. In this case it is
173appropriate but not required to print a short log message from the finalizer,
174as long as it is not expected to flood the logs.</p>
175<h2><a name="imports"></a>Imports</h2>
176<h3>Wildcards in imports</h3>
177<p><b>What it is</b>: When you want to use class Bar from package foo,there
178are two possible ways to import it:</p>
179<ol>
180<li><code>import foo.*;</code></li>
181<li><code>import foo.Bar;</code></li>
Dan Morrill3cd199f2009-11-06 14:04:16 -0800182</ol>
Dan Morrill1aad93f2010-05-28 14:49:47 -0700183<p><b>Pros of #1</b>: Potentially reduces the number of import statements.
Dan Morrill3cd199f2009-11-06 14:04:16 -0800184</p>
Dan Morrill1aad93f2010-05-28 14:49:47 -0700185<p><b>Pros of #2</b>: Makes it obvious what classes are actually used. Makes
186code more readable for maintainers. </p>
187<p><b>Decision</b>: Use style #2 for importing all Android code. An explicit
188exception is made for java standard libraries (java.util.*, java.io.*, etc.)
189and unit test code (junit.framework.*).</p>
190<h2><a name="javadoc"></a>Comments/Javadoc</h2>
191<p>Every file should have a copyright statement at the top. Then a package
192statement and import statements should follow, each block separated by a blank
193line. And then there is the class or interface declaration. In the Javadoc
194comments, describe what the class or interface does.</p>
195<pre>/*
196 * Copyright (C) 2010 The Android Open Source Project
197 *
198 * Licensed under the Apache License, Version 2.0 (the "License");
199 * you may not use this file except in compliance with the License.
200 * You may obtain a copy of the License at
201 *
202 * http://www.apache.org/licenses/LICENSE-2.0
203 *
204 * Unless required by applicable law or agreed to in writing, software
205 * distributed under the License is distributed on an "AS IS" BASIS,
206 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
207 * See the License for the specific language governing permissions and
208 * limitations under the License.
209 */
Dan Morrill3cd199f2009-11-06 14:04:16 -0800210
Dan Morrill1aad93f2010-05-28 14:49:47 -0700211package com.android.internal.foo;
212
213import android.os.Blah;
214import android.view.Yada;
215
216import java.sql.ResultSet;
217import java.sql.SQLException;
218
219/**
220 * Does X and Y and provides an abstraction for Z.
221 */
222public class Foo {
223 ...
224}</pre>
225<p>Every class and nontrivial public method you write <b>must</b> contain a
226Javadoc comment with at least one sentence describing what the class or method
227does. This sentence should start with a 3rd person descriptive verb.
228Examples:</p>
229<pre>/** Returns the correctly rounded positive square root of a double value. */
230static double sqrt(double a) {
231}
232
233/**
234 * Constructs a new String by converting the specified array of
235 * bytes using the platform's default character encoding.
236 */
237public String(byte[] bytes) {
238}</pre>
239<p>You do not need to write Javadoc for trivial get and set methods such as
240setFoo() if all your Javadoc would say is "sets Foo". If the method does
241something more complex (such as enforcing a constraint or having an important
242side effect), then you must document it. And if it's not obvious what the
243property "Foo" means, you should document it.</p>
244<p>Every method you write, whether public or otherwise, would benefit from
245Javadoc. Public methods are part of an API and therefore require Javadoc.</p>
246<p>Android does not currently enforce a specific style for writing Javadoc
247comments, but you <b>should</b> follow the <a
248href="http://java.sun.com/j2se/javadoc/writingdoccomments/">Sun Javadoc
249conventions</a>.</p>
250<h2><a name="shortmethods"></a>Short methods</h2>
251<p>To the extent that it is feasible, methods should be kept small and
252focused. It is, however, recognized that long methods are sometimes
253appropriate, so no hard limit is placed on method length. If a method exceeds
25440 lines or so, think about whether it can be broken up without harming the
255structure of the program.</p>
256<h2><a name="localvariables"></a>Local variables</h2>
257<p>The scope of local variables should be kept to a minimum (<i>Effective
258Java</i> Item 29). By doing so, you increase the readability and
259maintainability of your code and reduce the likelihood of error. Each variable
260should be declared in the innermost block that encloses all uses of the
261variable.</p>
262<p>Local variables should be declared at the point they are first used. Nearly
263every local variable declaration should contain an initializer. If you don't
264yet have enough information to initialize a variable sensibly, you should
265postpone the declaration until you do.</p>
266<p>One exception to this rule concerns try-catch statements. If a variable is
267initialized with the return value of a method that throws a checked exception,
268it must be initialized inside a try block. If the value must be used outside
269of the try block, then it must be declared before the try block, where it
270cannot yet be sensibly initialized:</p>
271<pre>// Instantiate class cl, which represents some sort of Set
272Set s = null;
273try {
274 s = (Set) cl.newInstance();
275} catch(IllegalAccessException e) {
276 throw new IllegalArgumentException(cl + " not accessible");
277} catch(InstantiationException e) {
278 throw new IllegalArgumentException(cl + " not instantiable");
279}
280
281// Exercise the set
282s.addAll(Arrays.asList(args));</pre>
283<p>But even this case can be avoided by encapsulating the try-catch block in a method:</p>
284<pre>Set createSet(Class cl) {
285 // Instantiate class cl, which represents some sort of Set
286 try {
287 return (Set) cl.newInstance();
288 } catch(IllegalAccessException e) {
289 throw new IllegalArgumentException(cl + " not accessible");
290 } catch(InstantiationException e) {
291 throw new IllegalArgumentException(cl + " not instantiable");
292 }
293}
294
295...
296
297// Exercise the set
298Set s = createSet(cl);
299s.addAll(Arrays.asList(args));</pre>
300<p>Loop variables should be declared in the for statement itself unless there
301is a compelling reason to do otherwise:</p>
302<pre>for (int i = 0; i n; i++) {
303 doSomething(i);
304}
305
306for (Iterator i = c.iterator(); i.hasNext(); ) {
307 doSomethingElse(i.next());
308}</pre>
309<h2><a name="import_style"></a>Imports</h2>
310<p>The ordering of import statements is:</p>
311<ol>
312<li>Android imports</li>
313<li>Imports from third parties (com, junit, net, org)</li>
314<li>java and javax</li>
315</ol>
316<p>To exactly match the IDE settings, the imports should be:</p>
317<ul>
318<li>Alphabetical within each grouping.</li>
319<li>Capital letters are considered to come before lower case letter (e.g. Z before a).</li>
320<li>There should be a blank line between each major grouping (android, com, junit, net, org, java, javax).</li>
Dan Morrill3cd199f2009-11-06 14:04:16 -0800321</ul>
Dan Morrill1aad93f2010-05-28 14:49:47 -0700322<h4>Why?</h4>
323<p>Originally there was no style requirement on the ordering. This meant that
324the IDE's were either always changing the ordering, or IDE developers had to
325disable the automatic import management features and maintain the imports by
326hand. This was deemed bad. When java-style was asked, the preferred styles
327were all over the map. It pretty much came down to our needing to "pick an
328ordering and be consistent." So we chose a style, updated the style guide, and
329made the IDEs obey it. We expect that as IDE users work on the code, the
330imports in all of the packages will end up matching this pattern without any
331extra engineering effort.</p>
332<p>The style chosen such that:</p>
333<ul>
334<li>The imports people want to look at first tend to be at the top (android)</li>
335<li>The imports people want to look at least tend to be at the bottom (java)</li>
336<li>Humans can easily follow the style</li>
337<li>The IDE's can follow the style</li>
338</ul>
339<h3>What about static imports?</h3>
340<p>The use and location of static imports have been mildly controversial
341issues. Some people would prefer static imports to be interspersed with the
342remaining imports, some would prefer them reside above or below all other
343imports. Additinally, we have not yet come up with a way to make all IDEs use
344the same ordering.</p>
345<p>Since most people consider this a low priority issue, just use your
346judgement and please be consistent.</p>
Dan Morrill3cd199f2009-11-06 14:04:16 -0800347
Dan Morrill1aad93f2010-05-28 14:49:47 -0700348<h2><a name="indentation"></a>Indentation</h2>
349<p>We use 4 space indents for blocks. We never use tabs. When in doubt, be
350consistent with code around you.</p>
351<p>We use 8 space indents for line wraps, including function calls and
352assignments. For example, this is correct:</p>
353<pre>Instrument i =
354 someLongExpression(that, wouldNotFit, on, one, line);</pre>
355<p>and this is not correct:</p>
356<pre>Instrument i =
357 someLongExpression(that, wouldNotFit, on, one, line);</pre>
358<h2><a name="field_names"></a>Field Names</h2>
359<ul>
360<li>Non-public, non-static field names start with m.</li>
361<li>Static field names start with s.</li>
362<li>Other fields start with a lower case letter.</li>
363<li>Public static final fields (constants) are ALL_CAPS_WITH_UNDERSCORES.</li>
364</ul>
365<p>For example:</p>
366<pre>public class MyClass {
367 public static final int SOME_CONSTANT = 42;
368 public int publicField;
369 private static MyClass sSingleton;
370 int mPackagePrivate;
371 private int mPrivate;
372 protected int mProtected;
373}</pre>
374<h2><a name="braces"></a>Braces</h2>
375<p>Braces do not go on their own line; they go on the same line as the code
376before them. So:</p>
377<pre>class MyClass {
378 int func() {
379 if (something) {
380 // ...
381 } else if (somethingElse) {
382 // ...
383 } else {
384 // ...
385 }
386 }
387}</pre>
388<p>We require braces around the statements for a conditional. Except, if the
389entire conditional (the condition and the body) fit on one line, you may (but
390are not obligated to) put it all on one line. That is, this is legal:</p>
391<pre>if (condition) {
392 body(); // ok
393}
394if (condition) body(); // ok</pre>
395<p>but this is still illegal:</p>
396<pre>if (condition)
397 body(); // bad</pre>
398<h2><a name="linelen"></a>Line length</h2>
399<p>Each line of text in your code should be at most 100 characters long.</p>
400<p>There has been lots of discussion about this rule and the decision remains
401that 100 characters is the maximum.</p>
402<p>Exception: if a comment line contains an example command or a literal URL
403longer than 100 characters, that line may be longer than 100 characters for
404ease of cut and paste.</p>
405<p>Exception: import lines can go over the limit because humans rarely see
406them. This also simplifies tool writing.</p>
407<h2><a name="annotations"></a>Java 1.5 Annotations</h2>
408<p>Annotations should precede other modifiers for the same language element.
409Simple marker annotations (e.g. &#64;Override) can be listed on the same line with
410the language element. If there are multiple annotations, or parameterized
411annotations, they should each be listed one-per-line in alphabetical
412order.</p>
413<p>Android -standard practices for the three predefined annotations in Java
4141.5's are:</p>
415<h3>&#64;Deprecated</h3>
416<p>The &#64;Deprecated annotation must be used whenever the use of the annotated
417element is discouraged. If you use the &#64;Deprecated annotation, you must also
418have a &#64;deprecated Javadoc tag and it should name an alternate implementation.
419In addition, remember that a &#64;Deprecated method is <b>still</b> supposed to
420work.</p>
421<p>If you see old code that has a &#64;deprecated Javadoc tag, please add the &#64;Deprecated annotation.</p>
422<h3>&#64;Override</h3>
423<p>The &#64;Override annotation must be used whenever a method overrides the
424declaration or implementation from a super-class.</p>
425<p>For example, if you use the &#64;inheritdocs Javadoc tag, and derive from a
426class (not an interface), you must also annotate that the method &#64;Overrides
427the parent class's method.</p>
428<h3>&#64;SuppressWarnings</h3>
429<p>The &#64;SuppressWarnings annotation should only be used under circumstances
430where it is impossible to eliminate a warning. If a warning passes this
431"impossible to eliminate" test, the &#64;SuppressWarnings annotation <b>must</b> be
432used, so as to ensure that all warnings reflect actual problems in the
433code.</p>
434<p>When a &#64;SuppressWarnings annotation is necessary, it must be prefixed with
435a TODO comment that explains the "impossible to eliminate" condition. This
436will normally identify an offending class that has an awkward interface. For
437example:</p>
438<pre>// TODO: The third-party class com.third.useful.Utility.rotate() needs generics
439&#64;SuppressWarnings("generic-cast")
440List&lt;String&gt; blix = Utility.rotate(blax);</pre>
441<p>When a &#64;SuppressWarnings annotation is required, the code should be
442refactored to isolate the software elements where the annotation applies.</p>
443<h2><a name="acronyms"></a>Acronyms in names</h2>
444<p>Treat acronyms and abbreviations as words. The names are much more readable:</p>
445<table><tbody>
446<tr><td>Good</td> <td>Bad</td></tr>
447<tr><td>XmlHttpRequest</td> <td>XMLHTTPRequest</td></tr>
448<tr><td>getCustomerId</td> <td>getCustomerID</td></tr>
449</tbody></table>
450<p>This style rule also applies when an acronym or abbreviation is the entire
451name:</p>
452<table><tbody>
453<tr><td>Good</td> <td>Bad</td></tr>
454<tr><td>class Html</td> <td>class HTML</td></tr>
455<tr><td>String url;</td> <td>String URL;</td></tr>
456<tr><td>long id;</td> <td>long ID;</td></tr>
457</tbody></table>
458<p>Both the JDK and the Android code bases are very inconsistent with regards
459to acronyms, therefore, it is virtually impossible to be consistent with the
460code around you. Bite the bullet, and treat acronyms as words.</p>
Dan Morrilla9788cd2009-11-15 11:49:30 -0800461<p>For further justifications of this style rule, see <i>Effective Java</i>
Dan Morrill1aad93f2010-05-28 14:49:47 -0700462Item 38 and <i>Java Puzzlers</i> Number 68.</p>
Dan Morrill3cd199f2009-11-06 14:04:16 -0800463
Dan Morrill1aad93f2010-05-28 14:49:47 -0700464<h2><a name="todo"></a>TODO style</h2>
465<p>Use TODO comments for code that is temporary, a short-term solution, or
466good-enough but not perfect.</p>
467<p>TODOs should include the string TODO in all caps, followed by a colon:</p>
468<pre>// TODO: Remove this code after the UrlTable2 has been checked in.
469
470// TODO: Change this to use a flag instead of a constant.</pre>
471<p>If your TODO is of the form "At a future date do something" make sure that
472you either include a very specific date ("Fix by November 2005") or a very
473specific event ("Remove this code after all production mixers understand
474protocol V7.").</p>
475
476<h2><a name="consistency"></a>Consistency</h2>
477<p>Our parting thought: BE CONSISTENT. If you're editing code, take a few
478minutes to look at the code around you and determine its style. If they use
479spaces around their if clauses, you should too. If their comments have little
480boxes of stars around them, make your comments have little boxes of stars
481around them too.</p>
482<p>The point of having style guidelines is to have a common vocabulary of
483coding, so people can concentrate on what you're saying, rather than on how
484you're saying it. We present global style rules here so people know the
485vocabulary. But local style is also important. If code you add to a a file
486looks drastically different from the existing code around it, it throws
487readers out of their rhythm when they go to read it. Try to avoid this.</p>
488
489<h2><a name="logging"></a>Logging</h2>
490<p>While logging is necessary it has a significantly negative impact on
491performance and quickly loses its usefulness if it's not kept reasonably
492terse. The logging facilities provides five different levels of logging. Below
493are the different levels and when and how they should be used.</p>
494
495<ul>
496<li><b>ERROR:</b>
497This level of logging should be used when something fatal has happened,
498i.e. something that will have user-visible consequences and won't be
499recoverable without explicitly deleting some data, uninstalling applications,
500wiping the data partitions or reflashing the entire phone (or worse). This
501level is always logged. Issues that justify some logging at the ERROR level
502are typically good candidates to be reported to a statistics-gathering
503server.</li>
Dan Morrill3cd199f2009-11-06 14:04:16 -0800504<li><b>WARNING:</b>
Dan Morrill1aad93f2010-05-28 14:49:47 -0700505This level of logging should used when something serious and unexpected
506happened, i.e. something that will have user-visible consequences but is
507likely to be recoverable without data loss by performing some explicit action,
508ranging from waiting or restarting an app all the way to re-downloading a new
509version of an application or rebooting the device. This level is always
510logged. Issues that justify some logging at the WARNING level might also be
511considered for reporting to a statistics-gathering server.</li>
Dan Morrill3cd199f2009-11-06 14:04:16 -0800512<li><b>INFORMATIVE:</b>
Dan Morrill1aad93f2010-05-28 14:49:47 -0700513This level of logging should used be to note that something interesting to
514most people happened, i.e. when a situation is detected that is likely to have
515widespread impact, though isn't necessarily an error. Such a condition should
516only be logged by a module that reasonably believes that it is the most
517authoritative in that domain (to avoid duplicate logging by non-authoritative
518components). This level is always logged.</li>
Dan Morrill3cd199f2009-11-06 14:04:16 -0800519<li><b>DEBUG:</b>
Dan Morrill1aad93f2010-05-28 14:49:47 -0700520This level of logging should be used to further note what is happening on the
521device that could be relevant to investigate and debug unexpected behaviors.
522You should log only what is needed to gather enough information about what is
523going on about your component. If your debug logs are dominating the log then
524you probably should be using verbose logging. This level will be logged, even
525on release builds, and is required to be surrounded by an if (LOCAL_LOG) or if
526(LOCAL_LOGD) block, where LOCAL_LOG[D] is defined in your class or
527subcomponent, so that there can exist a possibility to disable all such
528logging. There must therefore be no active logic in an if (LOCAL_LOG) block.
529All the string building for the log also needs to be placed inside the if
530(LOCAL_LOG) block. The logging call should not be re-factored out into a
531method call if it is going to cause the string building to take place outside
532of the if (LOCAL_LOG) block. There is some code that still says if
533(localLOGV). This is considered acceptable as well, although the name is
534nonstandard.</li>
Dan Morrill3cd199f2009-11-06 14:04:16 -0800535<li><b>VERBOSE:</b>
Dan Morrill1aad93f2010-05-28 14:49:47 -0700536This level of logging should be used for everything else. This level will only
537be logged on debug builds and should be surrounded by if (LOCAL_LOGV) block
538(or equivalent) so that it can be compiled out by default. Any string building
539will be stripped out of release builds and needs to appear inside the if
540(LOCAL_LOGV) block.</li>
Dan Morrill3cd199f2009-11-06 14:04:16 -0800541</ul>
Dan Morrill1aad93f2010-05-28 14:49:47 -0700542<p><i>Note:</i> Within a given module, other than at the VERBOSE level, an
543error should only be reported once if possible: within a single chain of
544function calls within a module, only the innermost function should return the
545error, and callers in the same module should only add some logging if that
546significantly helps to isolate the issue.</p>
547<p><i>Note:</i> In a chain of modules, other than at the VERBOSE level, when a
548lower-level module detects invalid data coming from a higher-level module, the
549lower-level module should only log this situation to the DEBUG log, and only
550if logging provides information that is not otherwise available to the caller.
551Specifically, there is no need to log situations where an exception is thrown
552(the exception should contain all the relevant information), or where the only
553information being logged is contained in an error code. This is especially
554important in the interaction between the framework and applications, and
555conditions caused by third-party applications that are properly handled by the
556framework should not trigger logging higher than the DEBUG level. The only
557situations that should trigger logging at the INFORMATIVE level or higher is
558when a module or application detects an error at its own level or coming from
559a lower level.</p>
560<p><i>Note:</i> When a condition that would normally justify some logging is
561likely to occur many times, it can be a good idea to implement some
562rate-limiting mechanism to prevent overflowing the logs with many duplicate
563copies of the same (or very similar) information.</p>
564<p><i>Note:</i> Losses of network connectivity are considered common and fully
565expected and should not be logged gratuitously. A loss of network connectivity
566that has consequences within an app should be logged at the DEBUG or VERBOSE
567level (depending on whether the consequences are serious enough and unexpected
568enough to be logged in a release build).</p>
569<p><i>Note:</i> A full filesystem on a filesystem that is acceessible to or on
570behalf of third-party applications should not be logged at a level higher than
571INFORMATIVE.</p>
572<p><i>Note:</i> Invalid data coming from any untrusted source (including any
573file on shared storage, or data coming through just about any network
574connections) is considered expected and should not trigger any logging at a
575level higher then DEBUG when it's detected to be invalid (and even then
576logging should be as limited as possible).</p>
577<p><i>Note:</i> Keep in mind that the '+' operator, when used on Strings,
578implicitly creates a StringBuilder with the default buffer size (16
579characters) and potentially quite a few other temporary String objects, i.e.
580that explicitly creating StringBuilders isn't more expensive than relying on
581the default '+' operator (and can be a lot more efficient in fact). Also keep
582in mind that code that calls Log.v() is compiled and executed on release
583builds, including building the strings, even if the logs aren't being
584read.</p>
585<p><i>Note:</i> Any logging that is meant to be read by other people and to be
586available in release builds should be terse without being cryptic, and should
587be reasonably understandable. This includes all logging up to the DEBUG
588level.</p>
589<p><i>Note:</i> When possible, logging should be kept on a single line if it
590makes sense. Line lengths up to 80 or 100 characters are perfectly acceptable,
591while lengths longer than about 130 or 160 characters (including the length of
592the tag) should be avoided if possible.</p>
593<p><i>Note:</i> Logging that reports successes should never be used at levels
594higher than VERBOSE.</p>
595<p><i>Note:</i> Temporary logging that is used to diagnose an issue that's
596hard to reproduce should be kept at the DEBUG or VERBOSE level, and should be
597enclosed by if blocks that allow to disable it entirely at compile-time.</p>
598<p><i>Note:</i> Be careful about security leaks through the log. Private
599information should be avoided. Information about protected content must
600definitely be avoided. This is especially important when writing framework
601code as it's not easy to know in advance what will and will not be private
602information or protected content.</p>
603<p><i>Note:</i> System.out.println() (or printf() for native code) should
604never be used. System.out and System.err get redirected to /dev/null, so your
605print statements will have no visible effects. However, all the string
606building that happens for these calls still gets executed.</p>
607<p><i>Note:</i> <b>The golden rule of logging is that your logs may not
608unnecessarily push other logs out of the buffer, just as others may not push
609out yours.</b></p>
Dan Morrill3cd199f2009-11-06 14:04:16 -0800610
Dan Morrill1aad93f2010-05-28 14:49:47 -0700611<h2>Javatests Style Rules</h2>
612<h2><a name="testmethodnames"></a>Naming test methods</h2>
613<p>When naming test methods, you can use an underscore to seperate what is
614being tested from the specific case being tested. This style makes it easier
615to see exactly what cases are being tested.</p>
616<p><a>For example:</a></p>
617<pre>testMethod_specificCase1 testMethod_specificCase2</pre>
Dan Morrill3cd199f2009-11-06 14:04:16 -0800618
Dan Morrill1aad93f2010-05-28 14:49:47 -0700619<pre>void testIsDistinguishable_protanopia() {
620 ColorMatcher colorMatcher = new ColorMatcher(PROTANOPIA)
621 assertFalse(colorMatcher.isDistinguishable(Color.RED, Color.BLACK))
622 assertTrue(colorMatcher.isDistinguishable(Color.X, Color.Y))
623}
Dan Morrill3cd199f2009-11-06 14:04:16 -0800624</pre>