Coding Philosophy and Best Practices
This evolving document contains our coding philosophy (general) followed by our best practices (specific and detailed). If you do not understand or agree with anything in the contents of this document, you should bring it up and discuss/debate it with fellow developers.
NOTE: THIS DOCUMENT IS A WORK IN PROGRESS. Some section titles are acting as place holders.
Software is an Engineering Discipline
We view software development as an engineering discipline wherein tradeoffs must be made between many competing “goods”. For example, there is a tradeoff between the cost to develop a piece of software and its reliability.
As engineers, we need to make these tradeoffs so as to maximize the value we deliver to our clients. Often this is difficult because the complexity and subtlety of the tradeoffs can be difficult to communicate properly to clients. In fact, in may respects, clear communication with the client about software problems is one of the most challenging aspects of being a software engineer.
Characteristics of Software Quality
Software has both external and internal qualities. External qualities are qualities that the client or software user would be aware of. Internal qualities are only apparent from the code (or perhaps data) itself. Of course, the line between these two is blurry, because internal qualities (e.g. no testing) will usually result in external qualities (poor reliability).
Here is a list of external qualities of a piece of software:
- Cost - How much did it cost to create and maintain?
- Correct - Does the software do what it was supposed to do?
- Usable - Is the software easy to use? There are many tradeoffs even within usability; what makes software easy for new users may conflict with what is easy for power users.
- Performant - Does the software make good use of system resource such as memory, disk space, network, process time?
- Reliable - Is the software bug-free?
- Secure - Does the software keep hackers and unauthorized users out of it?
- Adaptable - Can the system be altered to work without modification in different environments; usually this means that it is appropriately configurable.
- Robust - Does the system continue to work in the presence of invalid input or stressful environmental conditions.
Here is a list of internal qualities of a piece of software:
- Clarity - Can you easily tell what a piece of code is supposed to do? Can you look at it and predict its external qualities?
- Maintainable - How easy is it to keep the software working over time and to include new features?
- Portable - How easy is it to port the software to other environments (e.g. from Linux to Windows)
- Reusable - Can we save time and money on other projects by re-using the code?
- Readable - Is the code easy for developers to read and understand?
- Testability - Is it constructed in a way that the pieces can be easily and independently tested?
- Tested - Are there automated tests that document how the code should work, and allow us to easily test whether it does?
There are certainly other ways to segment the “quality landscape”. This particular segmentation includes some overlapping categories.
Setting up a development environment should be made as easy as possible. This is useful when you
- need to jump back into an old project
- need to setup an earlier version of a project to isolate a bug
- are blocked on an issue, and want to work on second one
- have “it works for me but not for you” bugs
- are pulling upstream changes with new dependencies or data migrations
- need “backup” from other developers for a deadline or vacation.
Standard Script Names
These standard scripts make it easy for developers to manage their project environments and start being productive quickly:
buildall— Build a fresh development environment. Should assume that system dependencies are present (e.g. Python, MySQL, Redis). Should NOT install system dependencies or isolation environments (e.g. python virtual environments). Should check system dependency versions. Should run within 5 minutes, and must run under 30 minutes. Should populate development databases with development data, if necessary.
build— Build things that are frequently rebuilt (e.g. typescript and SCSS). The
buildallscript should run this last, if it is present (some projects only need a
run— Start all of the development processes (e.g. the Django development server, Webpack hot module reloading, and the task runner).
test— Run the core set of tests. Should run within 5 minutes; must run within 10 minutes, move slow tests into
testall— Run all of the tests, even the slow ones.
clean— Clean all of the stuff that
cleanallscript should run this first, if it is present.
cleanall— Clean everything that
All of these commands except
run should be idempotent.
Note that not all of them are required. Also note that they are focused on development environments rather than production ones.
buildall script should create a minimal set of useful development data for new projects. This ensures developers don’t need to manually re-create data to test against.
The primary object of software architecture is to manage complexity.
There are several forms of documentation that go into a software project. Each form has its own place.
A comment with the form
TODO: Something I should address eventually are a standard format for indicating issues that should be addressed in the future. By using the same standard format, it makes it possible to
grep through a code base and find accumulated technical debt.
Write good commit messages:
- Separate subject from body with a blank line
- Limit the subject line to 50 characters
- Capitalize the subject line
- Do not end the subject line with a period
- Use the imperative mood in the subject line
- Wrap the body at 72 characters
- Use the body to explain what and why vs. how
There can be too many tests, although there are usually too few. Tests serve as live documentation for a codebase and provide protection from many accidentally-introduced bugs.
Programmatic end-to-end tests are a common source of technical debt accumulation; they are expensive to write and age quickly. Written end-to-end tests are a good compromise. Write out a list of test cases in English with clear, specific instructions.
All code should be reviewed by another Innolitics team member before being merged into the main branch.
Good code reviews take a lot of time for the reviewer, and while it may be frustrating to the author if there are delays, there are many upsides to reviews:
- Maintain code quality
- Ensure multiple people are familiar with all code
- An opportunity to praise strong craftsmanship
- A chance to learn from each other
Code reviews are often the last step in achieving a milestone or customer deliverable. Reviewers should give code reviews high priority when scheduling your time, and ideally will be completed within a few hours of being requested.
Bugs are inevitable, and they are as much the reviewer’s responsibility as the author’s.
When reviewing code, check for:
- Do they exit?
- Are they testing the right things?
- Are they the right types of tests?
- Does this affect any external documentation?
- Are appropriate comments updated?
- Are the git commit messages high quality?
- Release notes?
- Is the code easy to read?
- Could it be simplified further?
Typically code reviews should happen inside Pull Requests so that the communication can be kept around for future reference.
Lines SHOULD be shorter than 100 characters and MUST be shorter than 120 characters.
We try to avoid prescribing too much about using particular technologies because conventions change quickly.
Use Python 3 when possible.
Code Complete, by Steve McConnell, is about code construction. It is out-dated in that it implicitly assumes a “waterfall” development strategy and it does not discuss testing enough, however most of its suggestions are clear and relevant.
Clean Code, by Robert C. Martin, describes how to write clean readable code. Its suggestions overlap with Code Complete, but it is sufficiently different to warrant reading too. Some recommendations deal with Java-specific problems (e.g. the recommendation not to use arguments on class methods, and problems with argument ordering).