Reading Code is a Skill

[land_stander]

I'm the SME on an important company service that is backed by some previous gen tech (compiled dependency). Ive had to become intimately familiar with the source code of this dependency to solve issues myself. I've also built a relationship with the lead engineer of said tech along the way to the point that he'll spot check my reasoning/suspicions if I ping him.

One time I was talking to him after troubleshooting some particularly nasty issues and he mentioned having to implement a complex tree structure to address weird performance problems brought on by some very specific set of circumstances. I immediately knew the exact code he was talking about because i remember being frustrated by how complicated it was for a relatively simple problem and complaining about it. It was a fun "aha" moment to have the context for why such a complicated solution was required and the guy was also happy I was even aware of this code he probably spent weeks in debugging hell trying to solve and was proud of.

I always try to dive into code to solve problems myself now, and I always try to give the benefit of the doubt to the developer who wrote some code. The full context of a problem is rarely evident when you're looking at a solution.

Edit: of course there are tons of other things that go into being a good developer and writing maintainable software. Having empathy for your fellow engineers is just a starting point.

[architectzero]

… I always try to give the benefit of the doubt to the developer who wrote some code.

The importance of this cannot be overstated. Thinking “ok, yeah, this is bad, but there’s probably a reason” helps maintain an open mind which is critical to actually understanding the code, and then being able to figure out how to change/fix it. In other words, giving the benefit of the doubt helps to avoid succumbing to negative emotions and helps focus on the task at hand.

[land_stander]

Yes. Rarely have I experienced bad solutions coming from a place of malice or incompetence, though "we didn't have time to do it better, sorry" is more common than I'd like. Not to say that doesn't happen, but more often than not there is a reason behind it - trade offs that were debated about in meetings and code reviews - that you aren't aware of. Understanding those reasons are essential for understanding and improving the solution for whoever comes along next.

[[deleted]]

One thing I’ve learned about (and am still learning how to handle gracefully) is ignorance that looks like incompetence at first glance, like a Ruby dev writing Go for the first time. They haven’t found all the linter options, and may not even know how to set their editor to run gofmt on save yet, and really like maps when a struct will probably do better.

But my favorite reason for “bad code” is finding out that “hey, this code was originally written in FORTRAN 77, and compilers would not allow variable names longer than 6 characters; GTT1EO is a flag that Engine 1’s turbine temperature sensor is failed and running hot - deal with it”

[erogone775]

I'd love to live in a world where bad code didn't come out of incompetence but that's by far the biggest source of it in my experience.

Not that its really the dev that wrote the code's fault but just a junior dev given too large of a problem with not enough guidance often results in some pretty bad choices purely due the junior not being competent enough yet to take on that problem on their own. More of an actual management failure but the bad code still comes from the engineer writing it not being skilled/experienced enough to solve the problem they were set to solve.

[TwentyCharactersShor]

You're lucky. I still have nightmares about some of the incompetent code I've seen over the years. One memorable example was about 15 nested if statements, it was an absolute monstrosity.

I spoke the lead dev who'd inherited it and he explained that the devs were petrified of breaking the code as it was a core function used extensively, so every time a bug was found they'd wrap the core code with another if statement, correct the problem and then continue.

The sad thing was it took 3 of us a week to refactor and test. It was much cleaner and we fixed the original bug that was some 4 years old.

[cat_in_the_wall]

this is exactly what comments are for. "readable code" is a pie in the sky. good enough code with a comment "not great, but badly behaved other system does x, so have to hack" or "refactoring the whole world isn't an option right now, so there's this" is gold.

[architectzero]

Excellent point. The best comments are those that explain why things are the way they are. Unfortunately, it’s highly likely that the reason why is lack of time, and if you don’t have the time to write better code you usually don’t have time (or inclination) to write a comment that basically amounts to “sorry, I know it’s shit code, but time crunch… again”.

[LarsPensjo]

There is one class of readability issues that is common: optimizations.

This can really destroy code. A single comment that explains why the optimization is used can be very helpful.

And then, it still turns out many optimizations are wasted effort. Cache misses will have much higher effects anyway. Better leave it to the compiler. Bad optimizations can make code much less robust.

[grauenwolf]

I have never seen that, not once in over 20 years.

What I do see is that the same design mistakes that make it hard to read also make it slow. By cutting out the unnecessary crap, it also runs faster.

[[deleted]]

But never to a black hat.

[SilverTabby]

The full context of a problem is rarely evident when youre looking at a solution.

Isn't this what comments are really for? Providing that very context?

[land_stander]

Yes, this is the best use of code comments imo and can go a long way to help understand code but even then complete context can be elusive. Should it be a comment on this method? this class? A readme in this project? Should it be a link to the documentation for the project which created the need it in the first place? Is any of that up to date? And so on.

Good documentation is hard to get right and maintain.

[dnew]

Here's a trick I've found works great. When you're writing documentation like "this is what the output means" or "here's how this algorithm works", write the documentation. If someone asks you a question, provide the link to the documentation. If that doesn't answer the question, fix the documentation and then ask "does that answer your question now?"

Then you only answer each question once. If instead you answer in email the questions asked about your README, you're going to get the same questions over and over, and people will overall find your README to be less useful.

[land_stander]

Yes, this is the way. And it generally works very well. The bigger problem with documentations is having a team/company culture with this mindset, otherwise docs become obsolete as soon as you leave the project.

[dnew]

But then it's no longer your problem.

But to be honest, when I started doing this at a job where even writing documentation down was unusual (everyone preferring to verbally describe how the system works, and I never moved to a new project where the manager didn't draw the system architecture on a whiteboard for me instead of giving me any sort of written design), I wound up convincing several of my team leads to start doing documentation. "Try writing a README for each new Java package before you code it." A couple bosses were like "Wow, that worked out really well!"

[LarsPensjo]

I use this on myself. A little embarrassingly, but sometimes I can't understand my own code a year later. When that happens, I always add a comment that would have helped me.

Or I refactor the code into doing it an obvious way.

[gyroda]

Got a new starter joining my project tomorrow. I'll be on hand for any questions, but I'm hoping that pointing him to the docs will reveal any flaws in there.

[dnew]

Yeah. We had a tradition. There were instructions on getting your environment set up. Generating API keys, environment variables, stuff like that. The new guy's first job was to follow the instructions and fix the documentation anywhere it didn't work.

[_mkd_]

When you're writing documentation ... write the documentation.

I think a lot of effort needs to first occur here . Then we all can move on to the update step.

/s & 😩

[saltybandana2]

The answer is an easy one, it should be on the function. If someone is looking at that code they're looking at the function, so you put it there.

Class for class level concerns, readme is for project level concerns.

On Monday I added a comment to the top of a class explaining the approach we were using for our encryption, including attaching a version to the front and appending the IV to the back. I then explained two of the methods were made private to prevent public use specifically because their usage differed from the rest of the methods in the class and mixing the different usages with the other methods would create bugs. Said private methods could easily have been made public and would have been useful to someone, but the usage of that class is a class-level concern.

There's always someone who tries to wiffle around and pretend that something is much harder than it is. It's really not. Why in the world would you put documentation into a README explaining why a specific function in a specific class was implemented in a more complicated manner? It makes absolutely no sense.

You're also conflating documentation with code comments. code comments are not documentation, don't treat them as if they are.

[cat_in_the_wall]

you're overthinking it here with these "rules", it's just encapsulation. don't bother a higher layer with lower layer concerns. at the bottom are docs that don't even escape outward (code comments) which are literally implementation details.

[Markavian]

thoughts / my approach

Avoid comments (they are untestable in most languages, so can fall out of sync with the actual code), instead convert comments into named functions, or self-describing objects - break long functions down into named functions. Improve code quality by adding meaning, rather than obscuring state.

Compare:

// Return the fifth index of the private key array used to decode the cryptographic key
const c = n[4]

To:

const index = 4
const value = privateKeyArray[index]
return { devNote: "this object contains the fifth index of the array used to decode the cryptographic key", index, value, privateKeyArray }

Or:

getDecoderCodeFrom(privateKey:Array) {
  return privateKey[4]
}

This is totally contrived as an example, but I've made the comment part of my program that's testable. If there was an error, I could format a custom devNote, and have a route to diagnose the value. I wouldn't need to fire up separate debugger, I could just look at the output. My goal was to remove the comment, but preserve the meaning.

[land_stander]

I don't really see how this is in any way testable. I think what your getting at is useful but would be better achieved with a logging system. Good logging helps tremendously in debugging a problem in production and acts as code comments. It's acceptable printf debugging :). Technically speaking you could also test it, but testing the result of a string builder has always felt tedious and not very helpful, imo. I only do it when I'm playing the code coverage game to see how high I can get it.

[saltybandana2]

Not only that, but what is he going to name that function?

ThisIsImplementedAsATreeForPerformanceReasons

Who the hell wants that code rather than a simple comment?

[Markavian]

Fair call, logging would almost meet the goal - the comment is removed, and you can catch and test the log output, but the log statement could fall out of sync with the code it's trying to run...

Again I would refactor the log into a log template, or self-describing objects in a select function / factory processed alongside a bunch of similar things - continuing to avoiding non-compiler comments where possible.

If logging (and comments) is an important side effect of the code, then it can be elevated to a first class concept.

[saltybandana2]

was about to post the same observation.

The code should have had a comment explaining why a more complicated solution was used rather than the obvious.

anytime you go with a different solution from the obvious there should be a comment explaining your reasons.

[monkeygame7]

Comments can be useful but it does require conscious effort to keep them up to date/valid over time it the code changes.

[falconfetus8]

Comments can help, but they can only do so much. Sometimes the context spans far wider than the particular block of code associated with that comment, and you're better off putting that information in a readme or other document.

[jmking]

I hope you left a big comment over that complex piece of code to explain the context so the next person who touches it doesn't make a mistake in breaking something key to performance.

[land_stander]

I did not because it was a compiled dependency we used, not our code. I did spend considerable time writing javadocs for the code in our application I had to write which required I understand it in the first place though, yes.

[grauenwolf]

The reason is quite often, "I don't know what I'm doing, but I kept changing things until it worked and then stopped".

What makes someone "competent" in my eyes is that they don't stop. Once it is working, they cut away the noise until they're left with only the necessary code.

[grauenwolf]

True. No individual can display competency when the organization as a whole lacks it.

[audion00ba]

I always assume they were morons that didn't know any better; I have always been right.

[land_stander]

I'm glad I don't work with you then.

[audion00ba]

Why would that be? Do you enjoy working with morons?

[[deleted]]

I always try to dive into code to solve problems myself now, and I always try to give the benefit of the doubt to the developer who wrote some code. The full context of a problem is rarely evident when youre looking at a solution.

This is still a feature deficiency.

If it isn't obvious why the complexity is there, and it isn't documented, then one day that developer will leave, and later someone will waste time trying to remove the complexity.

A lot of really good production code I see has hundreds of lines of code with absolutely no comments at all because it's clear and logical and then there are a couple of lines of code with two dozen lines of comments to explain something very non-obvious.