Skip to content

Style Guide

Li Song edited this page Aug 31, 2022 · 3 revisions

Code Style

LSML avoids reinventing the wheel and uses the Google Java Style Guide. Formattinig is enforced by the use of the matching Google Java Format tool. This tool has plugins available for the blessed IDE (and maybe yours if differnt). The formatter can also always be invoked from the commandline with ./gradlew spottlesJavaApply. Commits with formatting errors will fail pre-merge checks preventing them from being merged, you can check if your commit will pass the checks with ./gradlew check.

Language

LSML uses British English. Comments, UI and other text and documentation shall use British English and shall strive to use correct punctuation, be crisp, and be free of spelling and grammatical errors.

Error Handling

LSML uses exceptions in the code base and as such code should alwas strive to have strong exception safety.

In general, errors that result from unexpected sources (file I/O, network errors, etc) result in exceptions that can be caught and handled at suitable levels in the code or let propagate all the way out to the default exception handler that will show an error dialogue to the user if they are severe enough. The application in general is robust enough that often one can keep using it even after unhandled exceptions are thrown but the user is always recommended to save their garage to a new file and restart the application.

Errors that are expected (user did something that shouldn't work) typically, but not always use return codes to indicate success or failure. In some cases where passing the return codes down a long call chain would create a lot of code, exceptions are used to simplify and strict exception safety is required. The Command system is one example of where this approach is taken to simplify the error handling.

(Un)Parity with PGI

It's an explicit non-goal of LSML to emulate all choices that have been made by PGI in their data files and UX. Instead LSML aims to provide a consistent developer experience where as far as possible everything is handled in a uniform way in the code and the UI is designed to be the best it can be without being beholden to following patterns established by PGI.

Best Practices

Through out this section please keep in mind the LSML is a big project, has a very low maintenance budget, and users have fast CPUs. It's much more important to reduce maintenance overhead by writing simple and clean code wherever possible, than to produce optimally fast code.

You should be familiar with these concepts:

  • DRY
  • SOLID
  • YAGNI
  • KISS
  • TDD
  • RAII
    • Most often associated with C++, in Java this means that constructors should produce usable objects. I.e. "initialize" methods are a code smell.

Some more best practices with less fancy acronyms:

Always use the smallest scope possible for variables. This makes it easier to reason about where and when a variable is used. This is a form of KISS.

Minimize (mutable) state

This is not as widely written about but it is equally important and a big part of KISS.

A unit test of a class method has 2^(input bits)*2^(class state bits) possible input combinations and 2^(return bits)*2^(class state bits) output combinations. To make testing easier we want to minimize the size of the input and the output as that directly influences how many tests we need to write. This is done by minimizing the number of inputs and the number of mutable class variables. Similarly to minimize scope, the less variables there are to consider, the easier it is to reason about the functionlity of a class.

Prefer Immutable Objects where it makes sense

Immutable objects is the ultimate form of minimizing mutable state. Since there is no mutable state, testing the class it self is much easier and testing uses of the class is also much easier. In Java, this is typically achieved by using Immutable Interfaces.

Push out conversions

This is something that isn't widely known but is a good habit to get into. When implementing a method and defininig its arguments, you should strive to never have type conversions in the class body. If you need a double then make the method take a double even if you know that the calling site will have the value as an int. Let the caller do the type conversion.

The motivation is that when implementing the method you can impossibly know what data type the current and future uses of the method may have available. So if you write code that has a conversion in the body that conversion is always there, even if the caller could have generated the argument with the correct type to begin with.

By consistently applying this rule, conversions will be pushed out as far as possible and in many cases avoided all together as at the callsite the code can be written to use the right types this is especially important for non-trivial types. It will reduce the risk of conversion errors (floating point truncation etc) and a side effect is that less conversions is less work. This is not to be conflated with premature optimisation, rather this is optimisation as a side effect of simpler code.

Prefer complex graphs of simple objects to simple graphs of complex objects

Testing a large and complex object that invariably has a large internal state (violating one of the earlier best practices) quickly becomes difficult and the maintenance burden increases rapidly as it becomes hard to reason about what the state of the class is like at any point in time. Contrast this with a complex graph of simple objects where you can test each object easily in isolation and mock out the simple objects in tests to isolate the behaviour you want to test of the complex graph. This is a form of "divide and conquer" approach applied to software design.

Readable code doesn't need comments

This is a bit of hyperbole but it carries some truth. There are several good articles about this on the web, but the TL;DR is: Comments should be used sparingly to say why, not what. However, please note that documentation is not a comment, it's documentation. If you find that you need to add a comment to say what is being done, consider breaking out a function with a descriptive name, or changing variable names, maybe even restructuring control flow to make it cleaner.

Prefer guard statements to nesting branches

Nesting of branch statements hurts readability as the different branches become separated by more code and get harder to keep track of. In addition, with deep nesting it's easy to mistakenly put the else branch on the wrong nesting level. Guard statements solve both problems by keeping the guard condition and the branch taken tightly togehter and by reducing nesting.

Optimize design and algorithms

LSML is a user driven desktop application with a slow update rate. Performance is rarely as issue, but maintenance is. When designing new code, make sure that the high level design and algorithms are correct and sufficient performance should follow naturally with readable code.

Optimisation on the source level should only be undertaken if there are benchmarks that indicate that it's necessary and higher level design and algorithm improvements are not possible. In particular pre-mature optimization at the cost of readability or best practices is frowend upon.

The Wikipedia article on Optimization is a good read.

In summary

While not an exhaustive list, there are other best practices as well, these are some of the more common and critical rules of thumb to follow when writing code in LSML. If properly applied these best practices should result in minimal code. And minimal code is good, the less code we write, the less code we have to maintain.