LGTM: Writing good enough code

on lgtm and code

This post is a written version of the internal talk I gave on writing good enough code

It is aimed at folks who are writing production code1 for the first time.

If you have been writing software for a while then you probably know these things, and you are not the target audience for this post 🙂

While doing code reviews with folks writing production code for the first time I observed few common suggestions, and later I gave an internal talk on the subject. This post is to written version of the talk.

Please keep in mind that these are some of the guidelines(not rules) one can keep in mind to write good enough code, and these are not complete in any way

What do I mean by good enough code?

Code which is easy to understand and gets the job done is good enough, code that meets the requirements is good enough, code that we can ship is good enough.

I believe mindful shipping beats perfection, and we can always iterate to make it better in future.

Our goal should be shipping as fast as we can while trying not to accumulate tech or product debt.

No Code

“Every line of code you write is time debt: it is another line that has to be debugged, another line that has to be supported, another line that may require a rewrite later, another line that might cause an interaction with a later feature”

– @patio11, source

PS: Not to be confused with no-code movement

DRY

  • “Do not Repeat Yourself” (DRY) with some WET code is considered a good practice
  • Keep things DRY, but don’t DRY too much because then it starts getting hard to read and reason about it.
  • DRY but don’t compromise readability to DRY something, readability takes precedence over DRYing in my books

Further Reading:

YAGNI

  • YAGNI - “You aren’t gonna need it”
  • Don’t assume you will need it (unless you are sure, and is part is requirements)
  • Don’t speculate how It would be used in far future and just write code that gets the job done for the current use-case

Single Responsibility

The Single Responsibility Principle (SRP) states that every class or module in a program should have responsibility for a single piece of that program’s functionality.

It means don’t write a method that does multiple unrelated things, for example, logging modules have a method to format dates, and that formatting feature is used all over the place.

And the same goes for methods, don’t write something that does more than two things. If you need to add some feature in method and when you feel it’s doing two things, refactor it

Write for Readability

If you only want to follow one thing, make it this, because readability matters, and the simple reason is that code is read more then it’s modified, and readable code will be easy to reason about, change, or replace when required.

“Programs should be written for people to read, and only incidentally for machines to execute.”

— Structure and Interpretation of Computer Programs

More time will be spent reading the code you write, you and your co-workers will be reading more code then you will write

Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live.

— John Woods

Further Reading:

No Deep Nesting

Deeply nested code is hard to read and hard to understand, it becomes very hard when you have to reason about it or debug it (extra hard when you are in middle of an outage), and as we mentioned readability matters, that’s why deep nesting is considered a code smell

A method with multiple nested if condition quickly becomes hard to comprehend, so try to keep nesting under control.

Use Guard Clauses to get rid of nested conditionals, and keep the branching factor low

Happy Path

The happy path is the optimistic path, It’s the path your code will take under normal conditions, It’s scenario featuring no exceptions or error conditions.

For example, validating input in the start and failing fast will allow you to write code which can work with the assumption that data (down the line) it has it valid since it was validated at the start.

It makes code more readable and easy to debug, also less branching because now you don’t need to handle invalid data

Fail Fast, Fail Early and Fail With Noise

Write your software such that, when there is a problem, it should fail as soon as it detects the problem and knows it can’t recover from it.

Failure should be visible to operator, and failing early is better than trying to proceed in a possibly unstable state where bad state trickles down.

Silent failures are one of the hardest kind of failure to debug, trace and fix, these failures leave no traces for operators, and leave people banging their heads against the wall

You should always leave a visible trace of your failure, raising an exception or logging based on the type of failures

Further Reading:

Shameless plug: I curate failuremodes.dev and failure modes newsletter, do check it out if you interested in how and why software systems fail

Return Single Type

One function should return only one type, eg. only string, hash, or floats. not float for one set of input and string for another set of input

If your function is returning different data types for different inputs then you might want to break it down into multiple functions or unify the returned data type.

Having multiple return types make it hard to test, and it can mean that we are not following single responsibility principle.

Multiple return type is considered a code smell, and should be avoided

Refactoring

It should not be left for later, refactor whenever you can, and when it feels the right thing to do.

Refactoring in small dosages keeps tech debt in check.

Closing Note

You want to ship faster but ship mindfully, and to ship faster you want to get your changes merged without lots of back and forth. You can do that by thinking and reasoning about how and why you are writing something before you start writing something.

Code less, engineer more by Liz Fong-Jones explains this very well, and It’s a must-read, this is something that I follow, and I try to not start coding till I understand the Why, What and How for the change

I suggest doing one round of code review yourself before the code review. I call this self-review, and I do it for all of my pull requests, docs, or anything that I share, and I always find things to improve, and I am sure you will also find things to improve too.

We should keep in mind that, these are guidelines, and they should not be taken to the extreme, our goal is to find a good balance, instead of bike-shading over these things guidelines, and we should focus on things that matter

Our goal is to solve problems and create value. our users don’t care if we are serving them using PHP, and jQuery or the hot new magical framework, as long as these things are not hindering user experience, your users don’t care.

Further Reading:

footnotes:

  1. any code that’s serving customers in any way (directly or indirectly) is production code in my books ↩


Continue Reading