Gray's Java Rules and Pet Peeves

Although I was a C hacker in a previous life, I've been primarily a Java programmer now for a couple of decades. I enjoy the strong types and the verbosity (yes you heard me) of the code. Although my Java "style" has changed over the years, I've come up with some guiding principles that I take seriously and try to never violate. These are mostly my opinion but I post them here in the [vain] hope that someone else might learn from them and take them somewhat to heart. Or maybe these are hear to remind future Gray when I stray.

The following is a living document, in no precise order, is my humble opinion, YMMV, best if eaten by date on package, etc..

• Never have a method name starting with "get", "set", or "is" unless they are getting or setting a field or some other trivial operation. foo.getValue() seems like a simple call until you realize that it does some massive calculation. That's very bad. Use other names such as calculateValue() or better verbs such as "assign", "retrieve", "calc", "access", etc.. And never have a get/is method have side-effects of modifying the object. No one is going to guess that someObject.getField() modifies the object. Again, use another verb.
• Use a code formatter often to standardize your indent levels and normalize your whitespace. I use Eclipse as my IDE and my muscle memory causes me to reformat my code on every save. If I'm working on other folks' code I certainly try to match their format as much as possible but my own code is strictly formatted. There are times with even the smartest formatter that the code looks like shit. In these cases I try hard to help the formatter and not just abandon it. I'll put a // comment at the end of a line to force a newline and do other things so the code still looks good after the formatter runs.
• I know I'm going to open the gates of hell here, but I'm a tab guy. People talk about tabs being different on everyone's terminals or IDEs but in my view that's the reason to use them. I use a 4 character tab stops and my code looks as good on the screen if someone uses 8 or 2. I believe that one of the reasons why tabs work for me is that I do no inner-line spacing. No lining up variable names or default values. No comments to the right of the line. The only place in my code where the tab character exists is on the left side of the line as the indent. This also goes hand-in-hand with the fact that I ask my IDE to format my code often (like every save). Tabs take up less bytes in the file (ok a ridiculous reason) and are easier to arrow over (1 keystroke to move indent levels).
• Mostly because of the previous point, shy away from in-line comments. Since I format often, inline comments tend to require the code to look more like a Word document or something with comment columns, etc.. Just. Say. No. I put them on the previous line indented with the code.
• If a method looks crappy with a ton of indents then try creating sub-methods. Sometimes it doesn't work if there are multiple returns or a crap-ton of necessary arguments but often this will help refactor a method and dramatically improve the code. Don't ever worry about the stack-frame. The JIT compiler will inline that method if possible/necessary.
• Just about all of the time we should be writing for maintainability as opposed to performance. Performance is important and we should always keep it in mind but I've seen a lot of highly complicated, unmaintainable code that folks argue is needed because of performance. Write. Clean. Code. If a profiler tells you that it is a performance problem then look at it again with performance in mind, but spaghetti code isn't good for anyone.
• I don't understand it when everyone talks about the need to reduce boilerplate. Boilerplate code is the easiest to generate -- usually a couple keystrokes in the IDE. I can debug boilerplate because there is file line-number that I can put a breakpoint on. Shocking. I can see where objects are being created which gives me a better idea of my memory bandwidth. Java 8 has some cool language features but I worry that its becoming easier and easier to generate code that you really don't know how it's being run and more importantly how many objects it's generating. I guess that's true of any high level language. Maybe I'll feel differently when I start using it more.
• Naming is important. Take the time with every package, class, field, method, etc. to name it appropriately. Even internal names are important because next month when you code back to your code, if you stumble over a name which isn't quite what the field holds, it will slow you down or cause you to misinterpret code. As with comments, you are writing clean, well-documented code for future you as much as you are for co-worker or another contributor around the world. Time spent on appropriate naming will save time in the long run. Often when I am struggling with a name, I will try to explain what it does to another programmer who doesn't know the specifics of my code. When I find myself saying something like "really what this field is doing is ...", the "..." is what it should be named.
• Choose Joda-time formatter over SimpleDateFormatter for thread-safe reasons. If you must use SimpleDateFormat then make sure to clone it first using the following pattern:

  private static final SimpleDateFormat DATE_FORMAT_TO_CLONE = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS"); ... SimpleDateFormat dateFormat = (SimpleDateFormat) DATE_FORMAT_TO_CLONE.clone(); String dateStr = dateFormat.format(new Date());

• Never make an explicit call to obj.toString() when chaining with "+" because it can generate NPEs. Replace
  "prefix" + number + obj.toString()
  with
  "prefix" + number + obj
  The implied StringBuilder.append(...) method will use the String.valueOf(obj) which will add "null" instead of throwing.
• Just about never use += with strings -- especially true in a loop. Each line of string + operators generates a StringBuilder instance. += generates and then throws away a builder for each line and should be replaced with something like the following which allows you to test and append.

  StringBuilder sb = new StringBuilder("initial optional value"); sb.append(" another string to append to the end of the last one"); if (someFieldIsTrue) { sb.append(" optional string is appended if some test passes"); } String final = sb.toString();

• Classes should always be in their own java file with the exception of inner-classes. Class names should start with an uppercase letter and be camel-cased.
• Constant fields should be at the top of the class, be final static (private if possible) and their names should be all capital letters with underscores between the words (ex: DEFAULT_NUM_FIELDS). I tend not to use all capitals for constant objects that I am going to be calling methods on, with the exception of strings. For example, I might have a field private final static Random random = new Random() but I would not make it all capitals. Something about mutating a "constant" object seems wrong. I'll let my IDE coloration identify it as static without saying it is constant.
• Normal fields should be below constants with injected fields (think Spring) ahead of local fields (think counters). Field names should start with a lowercase-letter and be camel-cased. Just say no to all Hungarian notation.
• Rarely if ever should class fields be public with the exception of constants that should be final static. Even constants should only be public if necessary.
• Methods should be below constructors, static, and abstract methods. Their names should start with a lowercase letter and be camel-cased.
• Fields should be usually at least 3 or 4 characters and should be self descriptive. You can make fields too long but usually the problem is too short. Anything that you have to stop and puzzle over should be changed to provide more detail. This applies also to local stack variables with the exception of loop variables which can be of the i, j, k variety. Shy away from pageC and openB type of names and replace with pageCount and opened.
• I do not like to provide javadocs on a method that is implementing one from an interface or base class. The base class should have the documentation and your IDE should be smart enough to show it to you. If there is something in the specific overridden implementation that needs to be pointed out then the abstract method should be referenced and the extra documentation provided.
• I'm a stickler for javadocs. Every class needs to have javadocs. Every non-private method should as well. I even have javadocs on private fields (usually single line) and methods if their usage isn't obvious or if it would help maintainability of complex code blocks. However, I don't put javadocs on get/set/is methods and hashcode/equals/toString when it should be obvious. I really don't like the default java8 lint which throws a fit if a get method doesn't have a javadoc. The idea that you need to take up screen real-estate to document getNumFields() is ridiculous. That said, your fields and especially your fields exposed with get/set/is should be well named and self-documenting. Be careful of names like count otherwise your callers will ask "count of what?"
• A real pet peeve is what I'll call "faux-comments". These are comments that show up on a method or field, take lines of screen and brain real-estate, trick you into thinking that you actually have comment, but they aren't adding anything to the code. Comments on obvious fields names are a case in point such as if the method name is calcAverage() and the comments say "Calculates the average". These are worse than having no comments at all. IDE javadoc templates often stick in the params and the exception and often people don't add anything there. I know that the method throws because I can read the code so don't take up space on the screen and in my brain just repeating it to me unless you want to tell me why it throws or in what circumstances. If you aren't going to add details to these auto-generated comments then remove them.
• Alarms always go off in my head when I see Integer or other object versions of primitives. Sometimes of course this is required but always be on the lookout for extraneous usage of them. The reason is that if someone calls Integer x; ... someMethod(x); and the method expects a primitive or even worse int y = x + 1, the line will throw a NullPointerException if x is null. These bugs are hard to see because there is no obvious object-period pattern at fault. Just something to watch for.
• If I have if/else blocks, I tend to make sure that a small block comes first. For example, if you have a large if block and a tiny else block, I will invert the test with a not (!) so that the tiny else block goes at the top in the if block instead so it is easier to see and it does not get lost.
• If I see a if (variable != null) { ... } else { ... } type of code then, barring a size difference between the if/else blocks (see previous), I will invert the test and the if/else blocks. I feel that != is slightly harder for my brain to process than == because if the number of syllables. I don't have any empirical proof of this but all things being equal I will make that change.
• I will almost always choose ArrayList over the LinkedList class. ArrayList is very tight efficient code and just about the only case that I would use LinkedList instead is when I want to remove elements frequently from the front of the list or otherwise reorganize the elements.
• Inner-classes should be at the bottom of the class below any of the outer class methods so that the implementations of the inner and outer classes do not get confused.
• Inner classes should be made static if at all possible. static inner-classes can live on their own and have no class connections to the outer class and therefore are less expensive. If you are only accessing a couple of fields in the outer class, consider passing them into the constructor for the inner class.
• Large code blocks are just about always suspicious. You should consider cutting out sub-method or 3 from the middle of a large block when the block size is over 100 lines. This certainly not a hard-and-fast rule and if you have a crap-ton of arguments to these sub-methods then it might not be worth it. One of the side effects of the sub-methods is that often the branching gets a lot easier because you can use return so you get some code simplification along with a good jump of readability. As always, make sure you choose a good name for your sub-method.
• I have my Eclipse IDE wrapping code lines at 120 characters these days. As our monitors have grown larger, this number has increases although I'm now worrying that my eyesight will get worse the number will go down. In any case, lines that go off to the right of screen aren't helpful if you have to scroll left/right as well as up/down to read your code. If you have a long string, putting a " + " in the middle of the string has no cost because the compiler will join the 2 sides so don't be afraid to wrap long strings too.
• Be really careful of gross levels of object chaining. This is another rule that can be ignored with you are doing StringBuilder appends in a chain or .withSetting(...).withAnotherSetting(...) type of patterns but creating a temporary variable with a good for the value you retrieve from a map will often make the code a ton easier as opposed to doing something like: map.get(key).someMethod(...). Even parsing that sort of line is hard let alone tracing down which object is null that caused a NPE. Just now I stumbled over something like the following because I was missing the ".get()" at the end of the line: producer.send(recoveryMessage).get(). Assigning the result of the send to a variable (here future) and then put the get on the next line was a lot cleaner.
• When dealing with boolean fields, I tend to not preface them with "is". That said, the method that gets the value of boolean fields does start with "is". I figure that the field should be terminated while the method should be isTerminated(). A lot of times I'll read the if or while statement to see if it makes sense since I figure at some level that's what my brain is doing. If I stumble over the wording then I figure at some level my understanding of the code is stressed. "if terminated" and "while not terminated" read fine to me.
• Never say if (boolField == true) or false. You should say if (boolField) instead. For me this is a readability thing. In my brain, it is easier to say if (terminated) as oppposed to if (terminated == true). Also if (terminated == false) is going to be very confusing because it starts with the opposite mental interpretation. if (!terminated) should be used because it will not have the same issues.

More to follow as things come to me.

Free Spam Protection   Android ORM   Simple Java Zip   JMX using HTTP   Great Eggnog Recipe