67 Bricks blog

This is the 7th part of a series about 67 Bricks’s coding principles. The previous posts are: 1, 2, 3, 4, 5 and 6.

The principle

Aim to write code for the present and for your knowledge of the near future, rather than on assumptions about what might happen

If any of these principles are to be taken in moderation and with pinches of salt, it’s this one, so let’s start with a caveat. I’m not saying “don’t plan ahead” or “don’t consider how the codebase might change over time”. However, it is important not to code based on guesses or assumptions. That is, not to code “just in case”.

That’s the general point, but let’s be more specific:

Don’t write code that you don’t know you need

The common adage here is YAGNI: You Aren’t Gonna Need It. It can be tempting to build a feature because it seems likely that it will be useful in the future. However, there are various downsides to this. The first is simply that we might be wrong, in which case we’ve spent time and effort on something that isn’t useful rather than on something that is. That non-useful code will still require maintenance and add to the overall size and complexity of the codebase unless or until the decision is made to remove it again, incurring more time and effort – what Martin Fowler calls the “cost of carry”.

Even if it does turn out to be a useful feature, building it before understanding the full requirements is risky. It’s likely we made incorrect assumptions that will still require correction later. Plus we spent that time building the unknown thing that could have been spent on something with a more immediate priority.

Don’t optimise code prematurely

Optimising code for speed or memory consumption or any other measure often comes with trade-offs in terms of readability and maintainability – not to mention that it takes development time. So heed the very common advice to avoid premature optimisation.

This isn’t to say code should never be optimised. If you have identified a performance problem in some part of an application, you absolutely should track down the source of the issue and improve it. But you should be sure there is a problem to fix first.

This also isn’t to say you should entirely disregard performance while writing code. There is a sensible middle ground between caring too much and too little about things like performance. A good general rule is that there are likely more important features of your code to care about in the first instance, like readability, maintainability and testability.

Don’t generalise code prematurely

Generalising code for reuse is an extremely important tool in the toolbox of creating a scalable and maintainable application. However, not all code needs to be reused. There can be a temptation to assume that a general and reusable solution is automatically the best one. But if you don’t know whether it will ever need to be reused, then it’s often better to stick to solving exactly and only the problem at hand.

As with optimisation, there are often trade-offs with generalising or genericising a class or function. Often it makes the code more abstract and harder to read or to reason about. It’s also easy to make incorrect assumptions about the right way to genericise or abstract code if you try to do it too early. After all, duplication is far cheaper than the wrong abstraction.

One common adage around this is the Rule of three, which boils down to the advice that you should only try to make a reusable function if the need comes up three times. Before that you should just write code for the case or cases that you have.

This is an example where caveats abound. There are cases where the costs of reuse are low enough and the costs of duplication are high enough that it make sense to genericise earlier. Nevertheless, the Rule of Three is a good rule of thumb to keep in mind and to only break when you’re convinced you have a good reason.

Don’t keep unused code around

Sometimes a class or function may become vestigial – in that it is no longer called by anything in the codebase – perhaps following a refactor or the removal of a feature. There can be a temptation to leave that code in place regardless – after all, it’s good code that serves a purpose even if it’s not currently a necessary purpose. We could leave it just in case the need comes back.

This is generally a bad idea. Uncalled code adds further clutter to a codebase that may already be intimidatingly large for a new starter or for someone trying to find their way around. But worse, uncalled code still requires maintenance. That uncalled code likely relies on other functions and therefore acts as deadweight, artificially inflating how much those other functions are used and how easy they might be to refactor or change. Once again, the YAGNI principle applies.

The same goes for leaving commented-out code in place just in case. A commented out block is likely simply to become out of date as the codebase evolves around it and leaves it behind.

If you’re ever in the position of being tempted to leave some unused or commented out code in place, remember that (I hope!) you use git or some equivalent version control system. So that code can always be retrieved in the history if it’s needed. But let’s be honest it probably won’t be.

Of course this advice needs to be modified if you’re writing a code library where the intent is to provide a range of classes or functions to other developers. In that case parts of your public API may not be called by any other code in the project. But the advice still applies to any code that isn’t part of the public API of the library.

---

Ultimately there are costs and benefits associated with every choice we make when writing an application. It’s tempting to only see the benefits of “just in case” code – more functionality, more optimised code, more flexible code – so it’s important to be cognizant of the costs too: things like time, readability, maintainability.

There are times when those benefits will outweigh the costs and also times when they won’t. The most important thing is to make sure we’re fully aware of what costs and benefits we’re weighing when we make these choices and to be aware of what assumptions we’re making in the process.

There are times also when it’s beneficial to try harder than usual to predict the future or to err more on the side of “better safe than sorry”. Data storage decisions can be especially hard to change later for example. In cases where you’re acquiring data from another source or from user behaviour, if you haven’t been storing some piece of information that a future feature requires, you may never be able to go back and fill it in for historical records. So some reasonable assumptions about the future may need to be made here. What “reasonable” means is unfortunately the kind of question that requires experienced judgement.

There are difficult balances to strike here. It’s sensible to plan for unexpected changes of requirements and to design your codebase so that you keep your options open for the unknowns of the future. But that flexibility has to be balanced against the other more immediate concerns we keep coming back to – readability, maintainability, testability. Always examine your assumptions and try to take account of all the costs and benefits when weighing these decisions.

References

YAGNI

The Rule of Three

The Wrong Abstraction

This is the 6th part of a series about 67 Bricks’s coding principles. The previous posts are: 1, 2, 3, 4 and 5.

The principle

Where reasonable, lean on reputable, existing libraries rather than writing your own solution for a problem that has already been solved.

There are a number of well known edicts in software engineering that relate to this principle. “Reinventing the wheel” describes writing your own software to solve a problem that has already been solved adequately. “Not Invented Here” (NIH) syndrome describes a tendency to believe that only self-authored code is valid and therefore to avoid using any third party software or libraries.

I’ve certainly been guilty of reinventing the wheel – sometimes because of NIH syndrome and sometimes because it didn’t occur to me to check whether the wheel had already been invented. I don’t think I’m alone in this.

At 67 Bricks, we encourage the use of libraries and third party software in our solutions because we recognise that our real goal is to create valuable products for our customers. And we can do that more effectively when we lean on existing solutions to solved problems.

It’s worth considering that the authors of a library are likely to have dedicated considerable time to their solution and to have encountered and fixed problems you haven’t thought of yet. Problems, big and small – from dropdown components to search engines – have endless complexities and edge cases. If we tie ourselves up on these details, we are not working on solving the real problem of the system we’re working on.

Jeff Bezos said a company should “focus on what makes your beer taste better” as an analogy to explain why a software company might use AWS rather than managing servers etc in house. This is a useful way to look at writing software. What makes our “beer taste better” is understanding our customers’ problems and designing systems and products that solve them. We are not directly making our “beer taste better” if we’re spending time reimplementing an HTTP server or a frontend component library.

However, it’s important to evaluate libraries before relying on them. Consider questions like how actively is it developed and maintained? Is it compatible with other tools in use? Does it have issues or bugs that might make it difficult to work with or cause problems for the larger project? Is its license compatible with the project? Is it accessible? Your colleagues and the community will have thoughts, recommendations and advice, so don’t be afraid to ask.

The infamous leftpad incident highlighted the inevitable vulnerability that comes with relying on 3rd party libraries. The specific issues with NPM that allowed that to be such a problem have since been addressed, but nevertheless it’s worth bearing this warning in mind when bringing in any dependency. We always want to make sensible choices about when to rely on a library and when not to. There’s a balance to find here – sometimes it weighs in favour of choosing the library despite that vulnerability and sometimes it doesn’t.

As with so many decisions, it comes down to a cost benefit analysis. But it’s important to be realistic about all the costs. There are potential costs of using a library: it may be badly supported or become deprecated, it may have or develop a security hole – or worse, have a security hole put in it maliciously. But there are potential costs of not using a library too, primarily time. Can you afford to spend hours, days or weeks implementing and then maintaining some piece of your system that has already been implemented – perhaps better – already?

So I would advise using libraries liberally where they add value and allow you to concentrate on solving more valuable problems, but remember to evaluate them adequately. Ultimately aim to find a balance between not reinventing the wheel while also not bringing in a new library for every little problem you find however small.

This is the 5th part of a series about 67 Bricks’s coding principles. The previous posts are: 1, 2, 3 and 4.

The principle

All code should be reviewed before it is merged into the main branch. All project members, however junior, should be involved in code reviewing.

This is another – I hope – uncontroversial principle.

At 67 Bricks we generally use Gitlab for our code hosting and use their merge request feature for signalling that a change is ready to review and then for checking the changes and leaving comments for the author. All the other big git hosting platforms have equivalent tools that are just as good, so there’s no excuse not to do code reviews when working in a team.

Code reviewing is beneficial for the quality of the codebase because a reviewer may spot edge cases, mistakes, issues or potential problems that the original author didn’t consider. They may also be able to suggest improvements, based on their own knowledge of the project or of the relevant technologies.

We all make mistakes, so having another pair of eyes looking over your work makes it more likely that those mistakes get noticed before they cause a real problem. We also all have different knowledge, experiences, strengths and weaknesses, so code reviewing is a way of bringing the experience of a second person to bear on a problem.

Another benefit is that the reviewer comes to the code at some distance from the detailed problems the developer had to wrangle with, and this can be useful when seeing the complete set of changes as a whole. This is distance that the author will probably gain themselves over the next days and weeks, but it’s useful to have it immediately from another person.

Knowing that your code will be reviewed also encourages you to be more thorough. This is just human behaviour, especially when we are busy and keen to be seen to be making progress.

Slightly less obviously, code reviewing also has benefits for the reviewer because it exposes them to areas of the codebase they may not have worked on before and encourages them to engage with, and constructively criticise, others’ code. This gives them the opportunity to be exposed to and learn from others’ approaches.

And I should emphasise constructive criticism. When it works well, code reviewing can lead to a closer team built on trust. When we, as reviewers, suggest changes, we need to do so without implying criticism. And when receiving review comments, we need to understand that the health of the codebase (and the project) is more important than our egos.

As much as it takes effort to do a good, thorough code review, the benefit is huge. I’m sure I can’t be the only person who has been guilty of waving through a review with minimal attention – perhaps because I know the author is sensible and writes good code – only to find later that it caused some bug that I could have prevented if I’d engaged with it a bit more. Skimping on the effort to review properly is a false economy because the mistakes you miss will just need to be fixed later, leading to more reviews.

Generally speaking we at 67 Bricks think only one person need review each change, but there may be cases where it makes sense for more than one person to be involved, for example to get the input of a particular subject matter expert.

I don’t think anyone would pretend that reviewing code changes is their favourite part of the job, but there are things we can do when putting our code up for review that make everyone’s lives easier.

• we can aim to keep merge requests small and focussed (spoiler alert: this is the focus of a future principle)
• we can provide any necessary context, descriptions and (if applicable) screenshots when opening the merge request to give the reviewer the best chance of understanding what they’re looking at
• we can aim to keep each commit focussed on a single, meaningful change rather than lumping lots of unrelated changes in each commit. This makes it easier to review commit by commit, which can be preferable in some cases
• we can be available to answer the reviewer’s questions. It can even be helpful to quickly walk a reviewer through your change so they fully understand the context and your intentions before fully reviewing it themselves

Code reviews can unfortunately lead to a bottleneck in the development process where a number of changes sit unreviewed and becoming stale while the team works on other things, so it’s worth trying to keep on top of them. It often works to have a team policy that, upon finishing a piece of work, members should review at least one open merge request before picking up something new.

Code reviewing generally isn’t what gets anyone up in the morning, but it’s immeasurably valuable for the overall quality of the codebase. And slacking on it is likely to lead to costlier problems later on, so it’s worth trying to do well.

Resources

https://leanpub.com/whattolookforinacodereview

https://conventionalcomments.org/

This is the 4th part of a series about 67 Bricks’s coding principles. The previous posts are: 1, 2 and 3.

The principle

Test at multiple levels

I don’t think it’s controversial to say that tests are A Good Thing.

Functionality should be well tested so that we can be confident that it works correctly. Tests at different levels bring different benefits and should be combined to provide a high level of confidence in the software’s quality.

A rough common rule of thumb is that there should be:

• lots of unit tests
  • these focus on individual units of code
  • they should be small, focused and quick to run
• slightly fewer integration tests
  • these focus on testing multiple units together, potentially including external systems like databases
  • they tend to be a bit slower to run and more involved to set up
• fewer again end-to-end tests
  • these test the whole stack
  • they generally test from the point of view of an end user or client of the system, so they might run via a headless browser or via a REST API
  • they tend to be comparatively slow and complex so they should be used sparingly and where they add real value
• a small number of smoke tests
  • these are very basic end-to-end tests that can be run post-deployment or at regular intervals to check that the service is healthy

There is much that sensible people can disagree on in the above, like where the line sits between unit and integration tests; how much value there is in mocking in unit tests and much more. But I think the broader point that there is value in having tests at multiple levels stands.

By writing good tests at multiple levels, and running them often, it is possible to have a high level of confidence that a piece of software is in good working order.

Well written tests can bring a huge number of benefits, some of which are perhaps less obvious than others.

Tests verify that a piece of functionality works as intended

This is perhaps the most obvious benefit of tests: they test that some code does what you think it does.

While at 67 Bricks we are fairly agnostic to TDD (you’re welcome to use it, but you don’t have to), we do advocate for interleaving writing code with writing tests, rather than writing all the code first and leaving the tests till the end. Writing tests throughout the process can be hugely helpful in writing code that does what you intend it to with minimal bugs.

Tests encourage good, clean, modular code

It is a good rule of thumb that if a unit of code is hard to test, it’s probably an indication of a problem that you should fix. If it’s hard to test, perhaps it’s too tightly coupled or it’s making some undue assumptions or it has a confusing interface or it’s relying on hard-to-reason-about side effects… Wherever the difficulty springs from, the fact that it’s hard to test is a useful warning sign.

Tests act as specifications of behaviour

Each of your tests can act as an encapsulated description of how this unit of code is intended to act in particular circumstances. This is great as a way of documenting the developers’ intentions. If I come to a method and find myself wondering what to expect if null is passed into it, then my life will be made a lot easier if there’s a corresponding test like

it('throws an error when null is passed in', () => {

This example uses jest, a popular testing framework in the Javascript/Typescript world that allows you to write very spec-friendly, descriptive test names. In languages or frameworks that require you to use function names rather than strings for test names, I advocate making those function names as long and descriptive as possible, like

void throwsAnErrorWhenNullIsPassedIn()

Tests act as examples of how to use units of code

Related to the above point, they also act as written examples to future developers of how to use or consume the module under test.

Tests guard against regressions and other unintended changes

One of the most valuable things about adding tests as new features develop is that they remain in the codebase indefinitely as guards against unintended behaviour changes in the future. When working on a large system – particularly when developers come and go over time – it’s invaluable to be able to get instant feedback that a change you’ve made has caused a test to fail. This is especially true if that test is descriptively named, as recommended above, because it will help you understand what to do to fix the failure. Perhaps your change has had unintended consequences, or perhaps the test simply needs updating based on your change – a well named and well written test will help you make that call.

For this reason, it can sometimes be useful to test fairly trivial things that wouldn’t be worth testing if the only benefit were checking that your code works. Sometimes it’s valuable to simply enshrine something in a test to prevent accidental changes to important behaviour.

Tests help you refactor with confidence

When refactoring, tests are indispensable. If the code is well covered by good tests, and those tests pass after you’ve finished the refactor, you can have a high degree of confidence that you haven’t inadvertently changed any behaviour.

Resources

https://martinfowler.com/articles/practical-test-pyramid.html

This is the 3rd part of a series about 67 Bricks’s coding principles. The previous posts are: 1 and 2.

The principle

Aim for simplicity over complexity. This applies to everything from overarching architectural decisions down to function implementations.

This principle is a close cousin of the previous one – aim for clear, readable code – but emphasises one particular aspect of what makes code clear and readable: simplicity.

Simpler solutions tend to be easier to implement, to maintain, to reason about and to discuss with colleagues and clients.

It can be tempting to think that for software to be good or valuable it must be complicated. There can be an allure to complexity, I think partly because we tend to equate hard work with good work. So if we write something labyrinthine and hard to understand, it’s tempting to think it must also be good. But this is a false instinct when it comes to software. In code, hard does not equal good. In general complexity for its own sake should be avoided. It’s important to remember that there’s absolutely nothing wrong with a simple solution if it does what’s needed.

There’s also value in getting a simple solution working quickly so that it can be demoed, reviewed and discussed early compared to labouring for a long time over a complex solution that might not be correct. Something we emphasise a lot working at 67 Bricks is the value of iteration in the agile process. It can be extremely powerful to implement a basic version of a feature, site or application so that stakeholders can see and play with it and then give feedback rather than trying to discuss an abstract idea. Here, simplicity really shines because often getting a simple thing in front of a stakeholder in a week can be a lot more valuable than getting a complicated thing in front of them in a month.

This principle applies at every level at which we work, from designing your architectural infrastructure, down through designing the architecture of each module in your system, down to writing individual functions, frontend components and tests. At every level, if you can achieve what you need with fewer moving parts, simpler abstractions and fewer layers of indirection, the maintainability of your whole system will benefit.

Of course there are caveats here. Some code has to be complicated because it’s modelling complicated business logic. Sometimes there must be layers of abstraction and indirection because the problem requires it. This principle is not an argument that code should never be complicated, because sometimes it is unavoidable. Instead, it is an argument that simplicity is a valuable goal in itself and should be favoured where possible.

Another factor that makes this principle deceptively tricky is that it is the system (the architecture, the application, the class etc) that should be simple, not necessarily each individual code change. A complex system can very quickly emerge from a number of simple changes. Equally, a complicated refactor may leave the larger system simpler. It’s important to see the wood for the trees here. What’s important isn’t necessarily the simplicity of an individual code change, but the simplicity of the system that results from it.

There’s also subjectivity here: what does “simple” really mean when talking about code? A good example of an overcomplicated solution is the FizzBuzz Enterprise Edition repo – a satirical implementation of the basic FizzBuzz code challenge using an exaggerated Enterprise Java approach, with layers of abstraction via factories, visitors and strategies. However, all the patterns in use there do have their purpose. In another context, a factory class can simplify rather than obfuscate. But it’s important not to bring in extra complexity or indirection before it’s necessary.

Resources

The Wrong Abstraction – Sandi Metz

The Grug Brained Developer

Simplicity is An Advantage but Sadly Complexity Sells Better

This is the 2nd part of a series about 67 Bricks’s coding principles. The first post, containing the introduction and first principle is here.

The principle

Aim for clear, readable code. Write clear, readable comments where necessary

You should make it a priority that your work be readable and understandable to those who might come to it after you. This means you should aim to write code that is as clear and unambiguous as possible. You should do this by:

• using clear variable, function and class names
• avoiding confusing, ambiguous or unnecessarily complicated logic
• adhering to the conventions and idioms of the language or technology you’re using

What can’t be made clear through code alone should be explained in comments.

Comments should focus on “why” (or “why not” explanations) far more than “how” explanations. This is particularly true if there is some historical context to a coding decision that might not be clear to someone maintaining the code in the future.

Note however that just like code, comments must be maintained and can become stale or misleading if they don’t evolve with the code, so use them carefully and only where they add value.

It is important to recognise that your code will be read far more times that it is written, and it will be maintained by people who don’t know everything you knew when you wrote it; possibly including your future self. Aim to be kind to your future self and others by writing code that conveys as much information and relevant context as possible.

I expect we’ve all had the experience of coming to a piece of code and struggling to understand it, only to realise it was you who wrote it a few months or weeks (or even days?) ago. We should learn from this occasional experience and aim to identify what we could have changed about the code the first time that would have prevented it. Better variable names? More comments? More comprehensive tests?

“You’re not going to run out of ink,” is something a colleague once commented on a pull request of mine to say that I could clarify the purpose of a variable by giving it a longer, more descriptive name. I think that’s a point worth remembering. Use as many characters as you need to make the job of the next person easier.

Of course, there’s some subjectivity here. What you see as obscure, someone else might see as entirely clear and vice versa. And certainly there’s an element of experience in how easily one can read and understand any code. The point really is to make sure that at least a thought is spared for the person who comes to the code next.

Examples

Here is an example that does not follow this principle:

const a = getArticles('2020-01-01');
a && process(a);

This example is unclear because it uses meaningless variable names and somewhat ambiguous method names. For example, it’s not clear without reading further into each method what they do – what does the date string parameter mean in getArticles? It also uses a technique for conditionally executing a method that is likely to confuse someone trying to scan this code quickly.

Now, here’s an example attempts to follow the principle:

// The client is only interested in articles published after 1st Jan
// 2020. Older articles are managed by a different system.
// See <ticket number>
const minDate = '2020-01-01';

const articlesResult = getArticlesSince(minDate);
if (articlesResult) {
  ingestArticles(articlesResult);
}

It provides a comment to explain the “why” of the hardcoded date, including relevant context; it uses much more meaningful names for variables and functions; and it uses a more standard, idiomatic pattern for conditionally executing a method.

Resources

Naming is Hard: Let’s Do Better (Kate Gregory, YouTube)