Best Practices

This evolving document contains our best practices for building medical imaging software. If you do not understand or agree with anything in it, bring it up in Slack so we can improve upon it.

Tradeoffs

Software engineering requires us to make tradeoffs; there are rarely situations where a best-practice or technology is always the right choice.

As engineers, we need to make tradeoffs to maximize the value we deliver to our clients. Making the right tradeoffs is difficult. We usually don’t know the full business context, and our clients usually don’t usually grasp the subtleties of the technical tradeoffs.

Software has both external and internal qualities. External qualities are visible to users. Internal qualities are only visible to developers. This distinction is often blurry because internal qualities often affect external qualities.

Here is a list of external qualities of a piece of software:

Here is a list of internal qualities of a piece of software:

There are certainly other ways to segment the “quality landscape”. This particular segmentation includes some overlapping categories.

Requirements

Most failed software projects are not due to technical failures. They are usually due to “requirements failures.” I.e., building the wrong thing.

We believe engineers should be involved with the requirements process. You need to know why you are building what you are building what you are building.

Requirements indicate what is needed.

Specifications indicate how the requirements should be met.

It is important to distinguish between requirements and specifications. Clients often will give us specifications. When this happens, avoid prematurely implementing these specifications. Rather, work backward to understand the requirements. Once you understand the requirements, you often will realize that they can be met much more easily and simplify using another approach (i.e., a different set of specifications).

As engineers, we often get excited about writing code. When a client or manager tells us what to write, we start imagining how we would implement it, and how much fun it will be. While writing code is more fun than having hard conversations with stakeholders about requirements, doing so can save immense amounts of time and effort. In the end, our success as individuals, our success as a company, and our impact on the world will depend on our value added.

Specifications

Once you know the requirements for a task, it is usually good to spend some time planning out what you are working on before jumping into the code. Here are some questions that are worth asking:

It is usually a good idea to write some tests first and to specify the API for the main functions and classes involved.

Development Environment

Motivation

Setting up a development environment should be made as easy as possible. This is useful when you

Standard Script Names

These standard development scripts make it easy for developers to manage their project environments and start being productive quickly:

All of these commands except run should be idempotent.

Test Data

The 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.

Complexity

The primary object of software architecture is to manage complexity.

Documentation

There are several forms of documentation that go into a software project. Each form has its own place. Documentation is expensive to write and maintain. It should be kept as short as possible and should only be written if it is useful. It is usually bad to write documentation about libraries or frameworks since this is duplicated effort; instead, link to other resources here. Documentation should accomplish things which can not be accomplished better in the code.

Comments

Comments are especially prone to going stale. Avoid them when possible, and prefer cleaning up bad code overwriting comments to explain it.

It is usually bad to commit commented-out-code.

Developers are usually hesitant to delete comments. Don’t be. If a comment isn’t useful, delete it.

TODOs

Use this form so that it is easy to grep: TODO: Something I should address eventually.

Commits

The smaller and newer a project is, the less useful commit messages are. There is a balance to be had.

Even if your commit messages are short, write good commit messages:

Testing

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.

End-to-End Tests

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.

Unit Tests

Unit tests should be small, isolated (and thus order-independent), and should be as small as possible.

Manual Tests

Sometimes, manual testing is more appropriate than automated testing. If something should be manually tested, write out simple step-by-step instructions for the manual test when you write the code.

Refactoring

Frequent small refactors are usually better than large refactors and re-writes.

It is usually best to avoid refactoring working code because you risk introducing bugs and you make the git history more complicated. If you are working in a file for other reasons, then it is good to improve the code. This may involve:

Avoid the urge to throw away and re-write code that you have not written. The value of rewriting code decreases the older and more widely used it is (and the more likely many subtle bugs have been fixed already).

Code Reviews

Usually, 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:

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:

Typically code reviews should happen inside Pull Requests so that the communication can be kept around for future reference.

Pointers for reviewers:

Pointers for people being reviewed:

Whitespace

Whitespace matters. Be consistent.

Lines SHOULD be shorter than 100 characters and MUST be shorter than 120 characters.

Technologies

We try to avoid prescribing too much about using particular technologies because conventions change quickly.

Choosing

Python

Follow PEP8.

Use Python 3 when possible.

References

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).