1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499
500
501
502
503
504
505
506
507
508
509
510
511
512
513
514
515
516
517
518
519
520
521
522
523
524
525
526
527
528
529
530
531
532
533
534
535
536
537
538
539
540
541
542
543
544
545
546
547
548
549
550
551
552
553
554
555
556
557
558
559
560
561
562
563
564
565
566
567
568
569
570
571
572
573
574
575
576
577
578
579
580
581
582
583
584
585
586
587
588
589
590
591
592
593
594
595
596
597
598
599
600
601
602
603
604
605
606
607
608
609
610
611
612
613
614
615
616
617
618
619
620
621
622
623
624
625
626
627
628
629
630
631
632
633
634
635
636
637
638
639
640
641
642
643
644
645
646
647
648
649
650
651
652
653
654
655
656
657
658
659
660
661
662
663
664
665
666
667
668
669
670
671
672
673
674
675
676
677
678
679
680
681
682
683
684
685
686
687
688
689
690
691
692
693
694
695
696
697
698
699
700
701
702
703
704
705
706
707
708
709
710
711
712
713
714
715
716
717
718
719
720
721
722
723
724
725
726
727
728
729
730
731
732
733
734
735
736
737
738
739
740
741
742
743
744
745
746
747
748
749
750
751
752
753
754
755
756
757
758
759
760
761
762
763
764
765
766
767
768
769
770
771
772
773
774
775
776
777
778
779
780
781
782
783
784
785
786
787
788
789
790
791
792
793
794
795
796
797
798
799
800
801
802
803
804
805
806
807
808
809
810
811
812
813
814
815
816
817
818
819
820
821
822
823
824
825
826
827
828
829
830
831
832
833
834
835
836
837
838
839
840
841
842
843
844
845
846
847
848
849
850
851
852
853
854
855
856
857
858
859
860
861
862
863
864
865
866
867
868
869
870
871
872
873
874
875
876
877
878
879
880
881
882
883
884
885
886
887
888
889
890
891
892
893
894
895
896
897
898
899
900
901
902
903
904
905
906
907
908
909
910
911
912
913
914
915
916
917
918
919
920
921
922
923
924
925
926
927
928
929
930
931
932
933
934
935
936
937
938
939
940
941
942
943
944
945
946
947
948
949
950
951
952
953
954
955
956
957
958
959
960
961
962
963
964
965
966
967
968
969
970
971
972
973
974
975
976
977
978
979
980
981
982
983
984
985
986
987
988
989
990
991
992
993
994
995
996
997
998
999
1000
1001
1002
1003
1004
1005
1006
1007
1008
|
<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>
<aside 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" class="external">Google
Java Style Guide</a>.
</aside>
<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 class="prettyprint">
void setServerPort(String value) {
try {
serverPort = Integer.parseInt(value);
} catch (NumberFormatException e) { }
}
</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 class="inline-block">
"<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" class="external">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 class="prettyprint">
void setServerPort(String value) throws NumberFormatException {
serverPort = Integer.parseInt(value);
}
</pre>
</li>
<li>
Throw a new exception that's appropriate to your level of abstraction.
<pre class="prettyprint">
void setServerPort(String value) throws ConfigurationException {
try {
serverPort = Integer.parseInt(value);
} catch (NumberFormatException e) {
throw new ConfigurationException("Port " + value + " is not valid.");
}
}
</pre>
</li>
<li>
Handle the error gracefully and substitute an appropriate value in the
<code>catch {}</code> block.
<pre class="prettyprint">
/** 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
}
}
</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 class="prettyprint">
/** 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);
}
}
</pre>
<aside 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.
</aside>
</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 class="prettyprint">
/** 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.
}
}
</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 class="prettyprint">
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!
}
</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>
Catch each exception separately as part of a multi-catch block, for example:
<pre class="prettyprint">
try {
...
} catch (ClassNotFoundException | NoSuchMethodException e) {
...
}</pre>
</li>
<li>
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.
</li>
<li>
Rethrow the exception. Many times you don't need to catch the
exception at this level anyway, just let the method throw it.
</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 class="prettyprint">
/*
* Copyright 2018 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 {
...
}
</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><strong>Examples</strong></p>
<pre class="prettyprint">
/** Returns the correctly rounded positive square root of a double value. */
static double sqrt(double a) {
...
}
</pre>
<p>or</p>
<pre class="prettyprint">
/**
* Constructs a new String by converting the specified array of
* bytes using the platform's default character encoding.
*/
public String(byte[] bytes) {
...
}
</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>
<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" class="external">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 class="prettyprint">
// 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));
</pre>
<p>
However, even this case can be avoided by encapsulating the try-catch
block in a method:
</p>
<pre class="prettyprint">
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));
</pre>
<p>
Loop variables should be declared in the for statement itself unless
there is a compelling reason to do otherwise:
</p>
<pre class="prettyprint">
for (int i = 0; i < n; i++) {
doSomething(i);
}
</pre>
<p>and</p>
<pre class="prettyprint">
for (Iterator i = c.iterator(); i.hasNext(); ) {
doSomethingElse(i.next());
}
</pre>
<h3 id="order-import-statements">Order import statements</h3>
<p>The ordering of import statements is:</p>
<ol>
<li>
Android imports
</li>
<li>
Imports from third parties (<code>com</code>, <code>junit</code>,
<code>net</code>, <code>org</code>)
</li>
<li>
<code>java</code> and <code>javax</code>
</li>
</ol>
<p>To exactly match the IDE settings, the imports should be:</p>
<ul>
<li>
Alphabetical within each grouping, with capital letters before lower
case letters (e.g. Z before a).
</li>
<li>
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>).
</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>
The imports people want to look at first tend to be at the top
(<code>android</code>).
</li>
<li>
The imports people want to look at least tend to be at the bottom
(<code>java</code>).
</li>
<li>
Humans can easily follow the style.
</li>
<li>
IDEs can follow the style.
</li>
</ul>
<p>
Put static imports above all the other imports ordered the same way as
regular imports.
</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.
</p>
<p><span class="compare-better">Recommended</span></p>
<pre class="prettyprint">
Instrument i =
someLongExpression(that, wouldNotFit, on, one, line);
</pre>
<p><span class="compare-worse">Not recommended</span></p>
<pre class="prettyprint">
Instrument i =
someLongExpression(that, wouldNotFit, on, one, line);
</pre>
<h3 id="follow-field-naming-conventions">Follow field naming conventions</h3>
<ul>
<li>
Non-public, non-static field names start with m.
</li>
<li>
Static field names start with s.
</li>
<li>
Other fields start with a lower case letter.
</li>
<li>
Public static final fields (constants) are ALL_CAPS_WITH_UNDERSCORES.
</li>
</ul>
<p>For example:</p>
<pre class="prettyprint">
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;
}
</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 class="prettyprint">
class MyClass {
int func() {
if (something) {
// ...
} else if (somethingElse) {
// ...
} else {
// ...
}
}
}
</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 class="prettyprint">
if (condition) {
body();
}
</pre>
<p>and this is acceptable:</p>
<pre class="prettyprint">if (condition) body();</pre>
<p>but this is not acceptable:</p>
<pre class="prettyprint">
if (condition)
body(); // bad!
</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 class="prettyprint">
// TODO: The third-party class com.third.useful.Utility.rotate() needs generics
@SuppressWarnings("generic-cast")
List<String> blix = Utility.rotate(blax);
</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 class="prettyprint">
// TODO: Remove this code after the UrlTable2 has been checked in.
</pre>
<p>and</p>
<pre class="prettyprint">
// TODO: Change this to use a flag instead of a constant.
</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>
<h4="log-sparingly-notes">Notes</h4>
<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 class="prettyprint">
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))
}
</pre>
</body>
</html>
|
