Lipstick on a Pig | Fredrb's Blog

This post was written at 2am by a sleep-deprived human

Lipstick on a pig

As humans, we have evolved to be very good at orienting ourselves in nature. A landscape, with mountains, rivers, and valleys is natural and we have a good memory for geographical references. And the same thing also happens with code - or so says Robert C. Martin. Therefore, our natural instinct is to remember the curvature of the code indentation, the syntax coloring in the rivers of characters, and the beautiful valleys between the scope blocks. This - according to Clean Coders - is not how we should be navigating code. Rather than relying on our natural hunter and gatherer instincts, we should name our functions and variables with meaningful names and keep them short - so short they should do one thing only. This way, I don’t have to memorize the mountains and valleys of the code and anyone joining the team can find their way through the code by simply reading the code. It’s like prose, not cartography.

When I got my first job as a Software Engineer over 10 years ago I was given a copy of Clean Code and Clean Coder by Robert C. Martin. I’ve read the books, watched the videos, attended trainings, and did TDD katas in my spare time. For the next couple of years I went around the corridors boasting about my professionalism for always writing tests and keeping functions to 10 lines at maximum. After a few years in the industry and reading code from many different codebases and programming languages, my view of the world became less black-and-white. You start reading different authors, get out of the bubble you’re in, and all of a sudden the absolutism starts falling apart. It’s like when you are told as junior “Do not repeat yourself!” but then after some years you realize that repeating yourself is sometimes fine - it’s certainly better than the wrong abstraction! Some of the best code I read had a lot of comments and sometimes a few hundred lines of code in a single function. The number of lines - turns out - wasn’t the problem. Mixing different concepts and abstractions layers was the real culprit!

The last couple of years I’ve developed a sense of taste. What good code looks like. You see many different shapes of functions and structs. Within a few minutes - I can tell how I feel about that code. I don’t know if it’s good or bad. If it solves the problem or not. But I can definitely tell if it matches or not my quality pattern. That doesn’t mean that the code that wrinkles my nose will prove to be bad, but the probability - after years of pattern recognition - is higher. I can’t help but think in mountains, rivers, and valleys. I know what a nice valley looks like compared to a swamp. I’ve learned, after years of trekking, what a good trail is. And if I am settling somewhere, I hope that I can identify fertile land when I see it.

It’s entirely subject to taste - but looking at the outline of these two pieces of code - I know which one I am going into with a higher expectation.

It’s 20 degrees Celsius at 9am and I’m biking to the office. It’s only an 8-minute ride. My company just opened a new office close to where I live. I haven’t worked in an office in a while. I have a pretty good setup at home - but after a while it’s nice to change the scenery. I get up the elevator. Pass the reception. Set up at my desk. Grab a cup of coffee. I’m ready to start my day. A couple new messages in my team’s slack channel. There is a pull request I need to review. It’s a pretty important change I was excited for. It’s not a large change, but it was thoroughly discussed before. We’ve had an RFC, a few discussions, gathered feedback, tested a few hypotheses, and finally landed on a good path forward.

I open the pull request - it’s shy of a thousand lines of code. Not all of it is actual code. There are some config files, some generated things, and a few test data files. It’s an end-to-end feature. Starts at the server and makes changes all the way down to our persistency layer. Nothing fundamental should change - but it’s still a highly impactful contribution. Let’s dig in!

We are adding a new endpoint to the API. Sure, that’s exactly what I expected.

Scroll scroll scroll

Ah, here is the core of it. Skimming through it looks good. I see there are a few comments explaining the change. That looks nice. Seems like the code is pretty polished already! That’s a lot of em-dashes though. But that’s fine. Some people like to use them — I do! Haven’t read the comments thoroughly yet, this is just a first pass. But skimming through these they seem to explain what is happening here.

Scroll scroll

There is a lot of logic here, I wonder why they implemented so much stuff. There is surely a reason for it. Seems like...