diff options
Diffstat (limited to 'en/setup/code-style.html')
| -rw-r--r-- | en/setup/code-style.html | 718 |
1 files changed, 718 insertions, 0 deletions
diff --git a/en/setup/code-style.html b/en/setup/code-style.html new file mode 100644 index 00000000..5367bd68 --- /dev/null +++ b/en/setup/code-style.html @@ -0,0 +1,718 @@ +<html devsite> + <head> + <title>AOSP Java Code Style for Contributors</title> + <meta name="project_path" value="/_project.yaml" /> + <meta name="book_path" value="/_book.yaml" /> + </head> + <body> + <!-- + Copyright 2017 The Android Open Source Project + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + --> + + + +<p>The code styles below are strict rules for contributing Java code to the +Android Open Source Project (AOSP). Contributions to the Android platform that +do not adhere to these rules are generally <em>not accepted</em>. We recognize +that not all existing code follows these rules, but we expect all new code to +be compliant.</p> + +<p class="note"><strong>Note:</strong> These rules are intended for the Android +platform and are not required of Android app developers. App developers may +follow the standard of their choosing, such as the <a +href="https://google.github.io/styleguide/javaguide.html">Google Java Style +Guide</a>.</p> + +<h2 id="java-language-rules">Java Language Rules</h2> +<p>Android follows standard Java coding conventions with the additional rules +described below.</p> + +<h3 id="dont-ignore-exceptions">Don't Ignore Exceptions</h3> +<p>It can be tempting to write code that completely ignores an exception, such +as:</p> +<pre><code>void setServerPort(String value) { + try { + serverPort = Integer.parseInt(value); + } catch (NumberFormatException e) { } +} +</code></pre> +<p>Do not do this. While you may think your code will never encounter this error +condition or that it is not important to handle it, ignoring exceptions as above +creates mines in your code for someone else to trigger some day. You must handle +every Exception in your code in a principled way; the specific handling varies +depending on the case.</p> +<p><em>Anytime somebody has an empty catch clause they should have a +creepy feeling. There are definitely times when it is actually the correct +thing to do, but at least you have to think about it. In Java you can't escape +the creepy feeling.</em> -<a href="http://www.artima.com/intv/solid4.html">James Gosling</a></p> +<p>Acceptable alternatives (in order of preference) are:</p> +<ul> +<li>Throw the exception up to the caller of your method. +<pre><code>void setServerPort(String value) throws NumberFormatException { + serverPort = Integer.parseInt(value); +} +</code></pre> +</li> +<li>Throw a new exception that's appropriate to your level of abstraction. +<pre><code>void setServerPort(String value) throws ConfigurationException { + try { + serverPort = Integer.parseInt(value); + } catch (NumberFormatException e) { + throw new ConfigurationException("Port " + value + " is not valid."); + } +} +</code></pre> +</li> +<li>Handle the error gracefully and substitute an appropriate value in the +catch {} block. +<pre><code>/** Set port. If value is not a valid number, 80 is substituted. */ + +void setServerPort(String value) { + try { + serverPort = Integer.parseInt(value); + } catch (NumberFormatException e) { + serverPort = 80; // default port for server + } +} +</code></pre> +</li> +<li>Catch the Exception and throw a new <code>RuntimeException</code>. This is +dangerous, so do it only if you are positive that if this error occurs the +appropriate thing to do is crash. +<pre><code>/** Set port. If value is not a valid number, die. */ + +void setServerPort(String value) { + try { + serverPort = Integer.parseInt(value); + } catch (NumberFormatException e) { + throw new RuntimeException("port " + value " is invalid, ", e); + } +} +</code></pre> +<p class="note"><strong>Note</strong> The original exception is passed to the +constructor for RuntimeException. If your code must compile under Java 1.3, you +must omit the exception that is the cause.</p> +</li> +<li>As a last resort, if you are confident that ignoring the exception is +appropriate then you may ignore it, but you must also comment why with a good +reason: +<pre><code>/** If value is not a valid number, original port number is used. */ +void setServerPort(String value) { + try { + serverPort = Integer.parseInt(value); + } catch (NumberFormatException e) { + // Method is documented to just ignore invalid user input. + // serverPort will just be unchanged. + } +} +</code></pre> +</li> +</ul> + +<h3 id="dont-catch-generic-exception">Don't Catch Generic Exception</h3> +<p>It can also be tempting to be lazy when catching exceptions and do +something like this:</p> +<pre><code>try { + someComplicatedIOFunction(); // may throw IOException + someComplicatedParsingFunction(); // may throw ParsingException + someComplicatedSecurityFunction(); // may throw SecurityException + // phew, made it all the way +} catch (Exception e) { // I'll just catch all exceptions + handleError(); // with one generic handler! +} +</code></pre> +<p>Do not do this. In almost all cases it is inappropriate to catch generic +Exception or Throwable (preferably not Throwable because it includes Error +exceptions). It is very dangerous because it means that Exceptions +you never expected (including RuntimeExceptions like ClassCastException) get +caught in application-level error handling. It obscures the failure handling +properties of your code, meaning if someone adds a new type of Exception in the +code you're calling, the compiler won't help you realize you need to handle the +error differently. In most cases you shouldn't be handling different types of +exception the same way.</p> +<p>The rare exception to this rule is test code and top-level code where you +want to catch all kinds of errors (to prevent them from showing up in a UI, or +to keep a batch job running). In these cases you may catch generic Exception +(or Throwable) and handle the error appropriately. Think very carefully before +doing this, though, and put in comments explaining why it is safe in this place.</p> +<p>Alternatives to catching generic Exception:</p> +<ul> +<li> +<p>Catch each exception separately as separate catch blocks after a single +try. This can be awkward but is still preferable to catching all Exceptions. +Beware repeating too much code in the catch blocks.</li></p> +</li> +<li> +<p>Refactor your code to have more fine-grained error handling, with multiple +try blocks. Split up the IO from the parsing, handle errors separately in each +case.</p> +</li> +<li> +<p>Rethrow the exception. Many times you don't need to catch the exception at +this level anyway, just let the method throw it.</p> +</li> +</ul> +<p>Remember: exceptions are your friend! When the compiler complains you're +not catching an exception, don't scowl. Smile: the compiler just made it +easier for you to catch runtime problems in your code.</p> +<h3 id="dont-use-finalizers">Don't Use Finalizers</h3> +<p>Finalizers are a way to have a chunk of code executed when an object is +garbage collected. While they can be handy for doing cleanup (particularly of +external resources), there are no guarantees as to when a finalizer will be +called (or even that it will be called at all).</p> +<p>Android doesn't use finalizers. In most cases, you can do what +you need from a finalizer with good exception handling. If you absolutely need +it, define a close() method (or the like) and document exactly when that +method needs to be called (see InputStream for an example). In this case it is +appropriate but not required to print a short log message from the finalizer, +as long as it is not expected to flood the logs.</p> + +<h3 id="fully-qualify-imports">Fully Qualify Imports</h3> +<p>When you want to use class Bar from package foo,there +are two possible ways to import it:</p> +<ul> +<li><code>import foo.*;</code> +<p>Potentially reduces the number of import statements.</p></li> +<li><code>import foo.Bar;</code> +<p>Makes it obvious what classes are actually used and the code is more readable +for maintainers.</p></li></ul> +<p>Use <code>import foo.Bar;</code> for importing all Android code. An explicit +exception is made for java standard libraries (<code>java.util.*</code>, +<code>java.io.*</code>, etc.) and unit test code +(<code>junit.framework.*</code>).</p> + +<h2 id="java-library-rules">Java Library Rules</h2> +<p>There are conventions for using Android's Java libraries and tools. In some +cases, the convention has changed in important ways and older code might use a +deprecated pattern or library. When working with such code, it's okay to +continue the existing style. When creating new components however, never use +deprecated libraries.</p> + +<h2 id="java-style-rules">Java Style Rules</h2> + +<h3 id="use-javadoc-standard-comments">Use Javadoc Standard Comments</h3> +<p>Every file should have a copyright statement at the top, followed by package +and import statements (each block separated by a blank line) and finally the +class or interface declaration. In the Javadoc comments, describe what the class +or interface does.</p> +<pre><code>/* + * Copyright (C) 2015 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.internal.foo; + +import android.os.Blah; +import android.view.Yada; + +import java.sql.ResultSet; +import java.sql.SQLException; + +/** + * Does X and Y and provides an abstraction for Z. + */ + +public class Foo { + ... +} +</code></pre> +<p>Every class and nontrivial public method you write <em>must</em> contain a +Javadoc comment with at least one sentence describing what the class or method +does. This sentence should start with a third person descriptive verb.</p> +<p>Examples:</p> +<pre><code>/** Returns the correctly rounded positive square root of a double value. */ +static double sqrt(double a) { + ... +} +</code></pre> +<p>or</p> +<pre><code>/** + * Constructs a new String by converting the specified array of + * bytes using the platform's default character encoding. + */ +public String(byte[] bytes) { + ... +} +</code></pre> +<p>You do not need to write Javadoc for trivial get and set methods such as +<code>setFoo()</code> if all your Javadoc would say is "sets Foo". If the method +does something more complex (such as enforcing a constraint or has an important +side effect), then you must document it. If it's not obvious what the property +"Foo" means, you should document it. +<p>Every method you write, public or otherwise, would benefit from Javadoc. +Public methods are part of an API and therefore require Javadoc. Android does +not currently enforce a specific style for writing Javadoc comments, but you +should follow the instructions <a +href="http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html">How +to Write Doc Comments for the Javadoc Tool</a>.</p> + +<h3 id="write-short-methods">Write Short Methods</h3> +<p>When feasible, keep methods small and focused. We recognize that long methods +are sometimes appropriate, so no hard limit is placed on method length. If a +method exceeds 40 lines or so, think about whether it can be broken up without +harming the structure of the program.</p> + +<h3 id="define-fields-in-standard-places">Define Fields in Standard Places</h3> +<p>Define fields either at the top of the file or immediately before the +methods that use them.</p> + +<h3 id="limit-variable-scope">Limit Variable Scope</h3> +<p>Keep the scope of local variables to a minimum. By doing so, you +increase the readability and maintainability of your code and reduce the +likelihood of error. Each variable should be declared in the innermost block +that encloses all uses of the variable.</p> +<p>Local variables should be declared at the point they are first used. Nearly +every local variable declaration should contain an initializer. If you don't +yet have enough information to initialize a variable sensibly, postpone the +declaration until you do.</p> +<p>The exception is try-catch statements. If a variable is initialized with the +return value of a method that throws a checked exception, it must be initialized +inside a try block. If the value must be used outside of the try block, then it +must be declared before the try block, where it cannot yet be sensibly +initialized:</p> +<pre><code>// Instantiate class cl, which represents some sort of Set +Set s = null; +try { + s = (Set) cl.newInstance(); +} catch(IllegalAccessException e) { + throw new IllegalArgumentException(cl + " not accessible"); +} catch(InstantiationException e) { + throw new IllegalArgumentException(cl + " not instantiable"); +} + +// Exercise the set +s.addAll(Arrays.asList(args)); +</code></pre> +<p>However, even this case can be avoided by encapsulating the try-catch block +in a method:</p> +<pre><code>Set createSet(Class cl) { + // Instantiate class cl, which represents some sort of Set + try { + return (Set) cl.newInstance(); + } catch(IllegalAccessException e) { + throw new IllegalArgumentException(cl + " not accessible"); + } catch(InstantiationException e) { + throw new IllegalArgumentException(cl + " not instantiable"); + } +} + +... + +// Exercise the set +Set s = createSet(cl); +s.addAll(Arrays.asList(args)); +</code></pre> +<p>Loop variables should be declared in the for statement itself unless there +is a compelling reason to do otherwise:</p> +<pre><code>for (int i = 0; i < n; i++) { + doSomething(i); +} +</code></pre> +<p>and</p> +<pre><code>for (Iterator i = c.iterator(); i.hasNext(); ) { + doSomethingElse(i.next()); +} +</code></pre> + +<h3 id="order-import-statements">Order Import Statements</h3> +<p>The ordering of import statements is:</p> +<ol> +<li> +<p>Android imports</p> +</li> +<li> +<p>Imports from third parties (<code>com</code>, <code>junit</code>, +<code>net</code>, <code>org</code>)</p> +</li> +<li> +<p><code>java</code> and <code>javax</code></p> +</li> +</ol> +<p>To exactly match the IDE settings, the imports should be:</p> +<ul> +<li> +<p>Alphabetical within each grouping, with capital letters before lower case +letters (e.g. Z before a).</p> +</li> +<li> +<p>Separated by a blank line between each major grouping (<code>android</code>, +<code>com</code>, <code>junit</code>, <code>net</code>, <code>org</code>, +<code>java</code>, <code>javax</code>).</p> +</li> +</ul> +<p>Originally, there was no style requirement on the ordering, meaning IDEs were +either always changing the ordering or IDE developers had to disable the +automatic import management features and manually maintain the imports. This was +deemed bad. When java-style was asked, the preferred styles varied wildly and it +came down to Android needing to simply "pick an ordering and be consistent." So +we chose a style, updated the style guide, and made the IDEs obey it. We expect +that as IDE users work on the code, imports in all packages will match this +pattern without extra engineering effort.</p> +<p>This style was chosen such that:</p> +<ul> +<li> +<p>The imports people want to look at first tend to be at the top +(<code>android</code>).</p> +</li> +<li> +<p>The imports people want to look at least tend to be at the bottom +(<code>java</code>).</p> +</li> +<li> +<p>Humans can easily follow the style.</p> +</li> +<li> +<p>IDEs can follow the style.</p> +</li> +</ul> +<p>The use and location of static imports have been mildly controversial +issues. Some people prefer static imports to be interspersed with the +remaining imports, while some prefer them to reside above or below all +other imports. Additionally, we have not yet determined how to make all IDEs use +the same ordering. Since many consider this a low priority issue, just use your +judgement and be consistent.</p> + +<h3 id="use-spaces-for-indentation">Use Spaces for Indentation</h3> +<p>We use four (4) space indents for blocks and never tabs. When in doubt, be +consistent with the surrounding code.</p> +<p>We use eight (8) space indents for line wraps, including function calls and +assignments. For example, this is correct:</p> +<pre><code>Instrument i = + someLongExpression(that, wouldNotFit, on, one, line); +</code></pre> +<p>and this is not correct:</p> +<pre><code>Instrument i = + someLongExpression(that, wouldNotFit, on, one, line); +</code></pre> + +<h3 id="follow-field-naming-conventions">Follow Field Naming Conventions</h3> +<ul> +<li> +<p>Non-public, non-static field names start with m.</p> +</li> +<li> +<p>Static field names start with s.</p> +</li> +<li> +<p>Other fields start with a lower case letter.</p> +</li> +<li> +<p>Public static final fields (constants) are ALL_CAPS_WITH_UNDERSCORES.</p> +</li> +</ul> +<p>For example:</p> +<pre><code>public class MyClass { + public static final int SOME_CONSTANT = 42; + public int publicField; + private static MyClass sSingleton; + int mPackagePrivate; + private int mPrivate; + protected int mProtected; +} +</code></pre> +<h3 id="use-standard-brace-style">Use Standard Brace Style</h3> +<p>Braces do not go on their own line; they go on the same line as the code +before them:</p> +<pre><code>class MyClass { + int func() { + if (something) { + // ... + } else if (somethingElse) { + // ... + } else { + // ... + } + } +} +</code></pre> +<p>We require braces around the statements for a conditional. Exception: If the +entire conditional (the condition and the body) fit on one line, you may (but +are not obligated to) put it all on one line. For example, this is acceptable:</p> +<pre><code>if (condition) { + body(); +} +</code></pre> +<p>and this is acceptable:</p> +<pre><code>if (condition) body(); +</code></pre> +<p>but this is not acceptable:</p> +<pre><code>if (condition) + body(); // bad! +</code></pre> + +<h3 id="limit-line-length">Limit Line Length</h3> +<p>Each line of text in your code should be at most 100 characters long. While +much discussion has surrounded this rule, the decision remains that 100 +characters is the maximum <em>with the following exceptions</em>:</p> +<ul> +<li>If a comment line contains an example command or a literal URL +longer than 100 characters, that line may be longer than 100 characters for +ease of cut and paste.</li> +<li>Import lines can go over the limit because humans rarely see them (this also +simplifies tool writing).</li> +</ul> + +<h3 id="use-standard-java-annotations">Use Standard Java Annotations</h3> +<p>Annotations should precede other modifiers for the same language element. +Simple marker annotations (e.g. @Override) can be listed on the same line with +the language element. If there are multiple annotations, or parameterized +annotations, they should each be listed one-per-line in alphabetical +order.</p> +<p>Android standard practices for the three predefined annotations in Java are:</p> +<ul> +<li><code>@Deprecated</code>: The @Deprecated annotation must be used whenever +the use of the annotated element is discouraged. If you use the @Deprecated +annotation, you must also have a @deprecated Javadoc tag and it should name an +alternate implementation. In addition, remember that a @Deprecated method is +<em>still supposed to work</em>. If you see old code that has a @deprecated +Javadoc tag, please add the @Deprecated annotation. +</li> +<li><code>@Override</code>: The @Override annotation must be used whenever a +method overrides the declaration or implementation from a super-class. For +example, if you use the @inheritdocs Javadoc tag, and derive from a class (not +an interface), you must also annotate that the method @Overrides the parent +class's method.</li> +<li><code>@SuppressWarnings</code>: The @SuppressWarnings annotation should be +used only under circumstances where it is impossible to eliminate a warning. If +a warning passes this "impossible to eliminate" test, the @SuppressWarnings +annotation <em>must</em> be used, so as to ensure that all warnings reflect +actual problems in the code. +<p>When a @SuppressWarnings annotation is necessary, it must be prefixed with +a TODO comment that explains the "impossible to eliminate" condition. This +will normally identify an offending class that has an awkward interface. For +example:</p> +<pre><code>// TODO: The third-party class com.third.useful.Utility.rotate() needs generics +@SuppressWarnings("generic-cast") +List<String> blix = Utility.rotate(blax); +</code></pre> +<p>When a @SuppressWarnings annotation is required, the code should be +refactored to isolate the software elements where the annotation applies.</p> +</li> +</ul> + +<h3 id="treat-acronyms-as-words">Treat Acronyms as Words</h3> +<p>Treat acronyms and abbreviations as words in naming variables, methods, and +classes to make names more readable:</p> +<table> +<thead> +<tr> +<th>Good</th> +<th>Bad</th> +</tr> +</thead> +<tbody> +<tr> +<td>XmlHttpRequest</td> +<td>XMLHTTPRequest</td> +</tr> +<tr> +<td>getCustomerId</td> +<td>getCustomerID</td> +</tr> +<tr> +<td>class Html</td> +<td>class HTML</td> +</tr> +<tr> +<td>String url</td> +<td>String URL</td> +</tr> +<tr> +<td>long id</td> +<td>long ID</td> +</tr> +</tbody> +</table> +<p>As both the JDK and the Android code bases are very inconsistent around +acronyms, it is virtually impossible to be consistent with the surrounding +code. Therefore, always treat acronyms as words.</p> + +<h3 id="use-todo-comments">Use TODO Comments</h3> +<p>Use TODO comments for code that is temporary, a short-term solution, or +good-enough but not perfect. TODOs should include the string TODO in all caps, +followed by a colon:</p> +<pre><code>// TODO: Remove this code after the UrlTable2 has been checked in. +</code></pre> +<p>and</p> +<pre><code>// TODO: Change this to use a flag instead of a constant. +</code></pre> +<p>If your TODO is of the form "At a future date do something" make sure that +you either include a very specific date ("Fix by November 2005") or a very +specific event ("Remove this code after all production mixers understand +protocol V7.").</p> + +<h3 id="log-sparingly">Log Sparingly</h3> +<p>While logging is necessary, it has a significantly negative impact on +performance and quickly loses its usefulness if not kept reasonably +terse. The logging facilities provides five different levels of logging:</p> +<ul> +<li><code>ERROR</code>: +Use when something fatal has happened, i.e. something will have user-visible +consequences and won't be recoverable without explicitly deleting some data, +uninstalling applications, wiping the data partitions or reflashing the entire +device (or worse). This level is always logged. Issues that justify some logging +at the ERROR level are typically good candidates to be reported to a +statistics-gathering server.</li> +<li><code>WARNING</code>: +Use when something serious and unexpected happened, i.e. something that will +have user-visible consequences but is likely to be recoverable without data loss +by performing some explicit action, ranging from waiting or restarting an app +all the way to re-downloading a new version of an application or rebooting the +device. This level is always logged. Issues that justify some logging at the +WARNING level might also be considered for reporting to a statistics-gathering +server.</li> +<li><code>INFORMATIVE:</code> +Use to note that something interesting to most people happened, i.e. when a +situation is detected that is likely to have widespread impact, though isn't +necessarily an error. Such a condition should only be logged by a module that +reasonably believes that it is the most authoritative in that domain (to avoid +duplicate logging by non-authoritative components). This level is always logged. +</li> +<li><code>DEBUG</code>: +Use to further note what is happening on the device that could be relevant to +investigate and debug unexpected behaviors. You should log only what is needed +to gather enough information about what is going on about your component. If +your debug logs are dominating the log then you probably should be using verbose +logging. +<p>This level will be logged, even on release builds, and is required to be +surrounded by an <code>if (LOCAL_LOG)</code> or <code>if (LOCAL_LOGD)</code> +block, where <code>LOCAL_LOG[D]</code> is defined in your class or subcomponent, +so that there can exist a possibility to disable all such logging. There must +therefore be no active logic in an <code>if (LOCAL_LOG)</code> block. All the +string building for the log also needs to be placed inside the <code>if +(LOCAL_LOG)</code> block. The logging call should not be re-factored out into a +method call if it is going to cause the string building to take place outside +of the <code>if (LOCAL_LOG)</code> block.</p> +<p>There is some code that still says <code>if (localLOGV)</code>. This is +considered acceptable as well, although the name is nonstandard.</p> +</li> +<li><code>VERBOSE</code>: +Use for everything else. This level will only be logged on debug builds and +should be surrounded by an <code>if (LOCAL_LOGV)</code> block (or equivalent) so +it can be compiled out by default. Any string building will be stripped out of +release builds and needs to appear inside the <code>if (LOCAL_LOGV)</code> block. +</li> +</ul> +<p><em>Notes:</em> </p> +<ul> +<li>Within a given module, other than at the VERBOSE level, an +error should only be reported once if possible. Within a single chain of +function calls within a module, only the innermost function should return the +error, and callers in the same module should only add some logging if that +significantly helps to isolate the issue.</li> +<li>In a chain of modules, other than at the VERBOSE level, when a +lower-level module detects invalid data coming from a higher-level module, the +lower-level module should only log this situation to the DEBUG log, and only +if logging provides information that is not otherwise available to the caller. +Specifically, there is no need to log situations where an exception is thrown +(the exception should contain all the relevant information), or where the only +information being logged is contained in an error code. This is especially +important in the interaction between the framework and applications, and +conditions caused by third-party applications that are properly handled by the +framework should not trigger logging higher than the DEBUG level. The only +situations that should trigger logging at the INFORMATIVE level or higher is +when a module or application detects an error at its own level or coming from +a lower level.</li> +<li>When a condition that would normally justify some logging is +likely to occur many times, it can be a good idea to implement some +rate-limiting mechanism to prevent overflowing the logs with many duplicate +copies of the same (or very similar) information.</li> +<li>Losses of network connectivity are considered common, fully expected, and +should not be logged gratuitously. A loss of network connectivity +that has consequences within an app should be logged at the DEBUG or VERBOSE +level (depending on whether the consequences are serious enough and unexpected +enough to be logged in a release build).</li> +<li>Having a full filesystem on a filesystem that is accessible to or on +behalf of third-party applications should not be logged at a level higher than +INFORMATIVE.</li> +<li>Invalid data coming from any untrusted source (including any +file on shared storage, or data coming through just about any network +connection) is considered expected and should not trigger any logging at a +level higher than DEBUG when it's detected to be invalid (and even then +logging should be as limited as possible).</li> +<li>Keep in mind that the <code>+</code> operator, when used on Strings, +implicitly creates a <code>StringBuilder</code> with the default buffer size (16 +characters) and potentially other temporary String objects, i.e. +that explicitly creating StringBuilders isn't more expensive than relying on +the default '+' operator (and can be a lot more efficient in fact). Keep +in mind that code that calls <code>Log.v()</code> is compiled and executed on +release builds, including building the strings, even if the logs aren't being +read.</li> +<li>Any logging that is meant to be read by other people and to be +available in release builds should be terse without being cryptic, and should +be reasonably understandable. This includes all logging up to the DEBUG +level.</li> +<li>When possible, logging should be kept on a single line if it +makes sense. Line lengths up to 80 or 100 characters are perfectly acceptable, +while lengths longer than about 130 or 160 characters (including the length of +the tag) should be avoided if possible.</li> +<li>Logging that reports successes should never be used at levels +higher than VERBOSE.</li> +<li>Temporary logging used to diagnose an issue that is hard to reproduce should +be kept at the DEBUG or VERBOSE level and should be enclosed by if blocks that +allow for disabling it entirely at compile time.</li> +<li>Be careful about security leaks through the log. Private +information should be avoided. Information about protected content must +definitely be avoided. This is especially important when writing framework +code as it's not easy to know in advance what will and will not be private +information or protected content.</li> +<li><code>System.out.println()</code> (or <code>printf()</code> for native code) +should never be used. System.out and System.err get redirected to /dev/null, so +your print statements will have no visible effects. However, all the string +building that happens for these calls still gets executed.</li> +<li><em>The golden rule of logging is that your logs may not +unnecessarily push other logs out of the buffer, just as others may not push +out yours.</em></li> +</ul> + +<h3 id="be-consistent">Be Consistent</h3> +<p>Our parting thought: BE CONSISTENT. If you're editing code, take a few +minutes to look at the surrounding code and determine its style. If that code +uses spaces around the if clauses, you should too. If the code comments have +little boxes of stars around them, make your comments have little boxes of stars +around them too.</p> +<p>The point of having style guidelines is to have a common vocabulary of +coding, so people can concentrate on what you're saying, rather than on how +you're saying it. We present global style rules here so people know the +vocabulary, but local style is also important. If the code you add to a file +looks drastically different from the existing code around it, it throws +readers out of their rhythm when they go to read it. Try to avoid this.</p> + +<h2 id="javatests-style-rules">Javatests Style Rules</h2> +<p>Follow test method naming conventions and use an underscore to separate what +is being tested from the specific case being tested. This style makes it easier +to see exactly what cases are being tested. For example:</p> +<pre><code>testMethod_specificCase1 testMethod_specificCase2 + +void testIsDistinguishable_protanopia() { + ColorMatcher colorMatcher = new ColorMatcher(PROTANOPIA) + assertFalse(colorMatcher.isDistinguishable(Color.RED, Color.BLACK)) + assertTrue(colorMatcher.isDistinguishable(Color.X, Color.Y)) +} +</code></pre> + + </body> +</html> |