If there's one thing I've learned about coding on a large project with a large team, it's that coding style is actually quite important. This article on Wikipedia explains well why consistent coding conventions are important.
Now it's also true that much of the code I've written for War Worlds isn't exactly consistent in every aspect. That's just an artifact of the fact that I've been working alone for the most part and not too worried about these things. But I think this needs to change.
To that end, I'm going to suggest we start with the Google coding conventions for the Java language.
There's a couple of places where the current coding style of War Worlds differs from the Google style, but I'd like to move over time to make the whole source tree in line with the Google style guide.
Main aspects of the style guide I want to stick to
Eventually, I'd like the whole source tree to be in line with the Google style guide, with the following exceptions:
- Section 3.1, there is no copyright statement in War Worlds source files (copyright is handled by the top-level LICENSE file).
- Section 3.3.3, au.com.codeka imports take the place of com.google imports at the top of the import section.
- Section 4.3.2, the order of methods in Activity/Fragment classes follows the approximate order the methods are called in the lifecycle. (i.e. onCreate -> onStart -> onResume -> onPause -> onStop -> onDestroy).
- Section 4.4, 100 characters is the column limit.
Some additional conventions I'd like to stick to
In addition to those in the Google style guide, I'd also like to stick to the following:
- Use @Nullable and @NonNull annotations for member variables and parameters that you expect may be null. This allows us to use Eclipse's null-analysis to help detect potential null reference exceptions
- Use Guava libraries sparingly in the client, extensively in the server. Guava is not really optimized for Android's limitations around GC and memory, so many of it's classes don't play well there. But it is still quite useful, so judicious use is still a good idea I think. In the server, there's no reason not to make full of Guava.
- Logging is always a tough thing to get right. Too much and it's useless noise. Not enough and you can miss vital information in crash reports and so on. Always use our Log class for logging (so that we're using common logging between client & server), and use best judgment to decide between "debug" and "info" level. Generally, "info" will be available in production, and "debug" will be disabled.
A lot of the existing code is not following these conventions. In particular, much of the existing code is using 4 space indents and prefixing member variables with "m" (this is the "Android" style). My general recommendation would be to stick with the existing style if you're not making exenstive changes, and convert the whole file at once to the new style if you're making significant changes to it anyway.
I'll try to ensure you're sticking with the above coding conventions before accepting a pull request. Don't take it personally :-)