Code Reviews – Ways to Explain Code When It Doesn’t Make Sense

code-reviewsreactjstypescript

As a programmer I have found my code frequently elicits the reaction "I don't understand". Whenever I get this response I try my best to explain my code patiently, and not make anyone feel afraid to ask questions.

I am pretty sure I've got the second part right, people are certainly not afraid to ask questions about my code!

However I have good reason to believe my explanations are not effective. I routinely have hour long discussions trying to explain my code, and on many occasions the conversations have ended with my coworker saying they still don't understand, but they have somewhere else to be (lunch, or home, or a meeting, etc).

I believe this is a problem with my code, as I cannot recall the last time some else's code has taken an hour of explanation to understand. As well, I rarely see my coworkers spending anywhere near as much time explaining their code to each other.

Specifically, when posed with the question "I don't understand your code", what are some strategies I can use to explain my code?

I have previously employed the following follow up questions, and I am looking for better, or at least more, follow questions:

  • What part specifically seems to be confusing?
    • Sometimes this works, but often the answer is "the whole thing". I have been in meetings with 5 other programmers, where all of the programmers agreed they didn't understand my code, but none of them could give any specifics parts which were confusing.
  • Are you familiar with pattern "X"?
    • I have tried to learn the names of the coding patterns I tend to use. I will bring up these names, such as "the visitor pattern", and ask them if they are familiar with this pattern. If they are familiar with it I try to show them how my code is an implementation of that pattern. This seems to stop them from immediately asking more questions, but invariably we seem to come back to the same code, and so I am afraid while they fully understand the pattern, the connection between the pattern and my code is not obvious.
  • What are some solutions to problem "X"?
    • Sometimes I try to get them to actively engage with solving the general problem, hoping that if they explain how they would solve it, I can show them the parallels between their solution and mine. This works, however often times the problem is a bit too complicated to just solve in your head, and so they can't quickly describe how they would solve it.

ADDITIONAL INFORMATION:

The code I work on most frequently is framework/architectural code, often legacy code which no one presently with the company is familiar with. My team is very busy, and while they are patient, they honestly don't have the time to help me work through legacy code. As a result my approach as been to fully understand it myself, and then try to explain it to my team during team meetings.

They will be interfacing with it though, and they interface with the existing code on a daily basis.

An example of this type of code would be our log pipeline, which takes browser errors, server errors, service errors, http logs, javascript logs, web logs and correctly joins time with session information, going through a few steps before the data eventually ends up in splunk. It isn't exactly complicated, but it also isn't exactly trivial, as the servers needed to handle tens of millions of logs per day, without any significant impact on server performance (our servers are already more expensive than my yearly salary).


CODE SAMPLES

(Please excuse the text dump. I tried to keep it short, but code samples seem like the best way to demonstrate my problem).

I put together a code sample of one piece of code that seemed to confuse my teammates the most. I no longer work at the company, so it isn't the exact code, and the exact code was scrapped anyway (it confused everyone, so we all agreed no one should use it).

A bit of background, our company was beginning a major rewrite, converting from a legacy framework to React/Typescript/Redux. There were regretting using Redux, but due to our browser support restrictions we were unable to use Mobx. As a result we were using Redux poorly, trying to make it work like Mobx, or KnockoutJS. The majority of our reducers simple set state, with the caller knowing exactly what they wanted to set (not how Redux action/reducers should work). However due to time constraints, we simply could not switch frameworks, and had to make Redux work. That was at least 3-4 years ago, and I would be surprised if the team was still using Redux now.

(I've linked to the Typescript playground for my code, as it is a bit long for a question)

An example of existing code can be found here: original code

I am opposed to this style, as although it is clear, it requires changing 4 pieces of code (spread across 3 different files) to add a variable. The steps to adding a new variables are: update the state definition, add a new action, add to the actions union, and add a reducer handler.

I made a builder class (a term I may not be using correctly, basically it is like yargs, https://www.npmjs.com/package/yargs, where you make a series of chained function calls to create a more complex object) that makes it possible to only add properties to one place, while preserving the types of everything.

(This was before Typescript mapped types, which provides alternatives to the builder approach).

A recreation of my proposed code can be found: changed code

Best Answer

Siloed

Framework and Infrastructure code is tricky. It is the dark and messy parts of the code base that hit actual walls, and the worst part is that often the solutions are counter-intuitive, as they have to work around the user (aka. programmer), language decisions, and platform idiosyncrasies.

What has happened is that you've become an expert, and become effectively siloed.

The worst part is this kind of code does not have an effective boundary between your code and the users code.

There are a few ways to deal with this situation.

Share the Pain

Nothing breeds knowledge quite like having to shovel the S#*T yourself.

Not everyone on the team will have the head for infrastructure/framework work, but there will be a few. The best way to start distributing knowledge is to start getting these developers to work on small areas of the infrastructure/framework.

Of course maintain oversight (it is critical after all), but you need to start getting the other developers thinking across the silo border.

Enforce the Border

If for one reason or another tearing down the silo cannot be done. The other strategy is to enforce better boundaries between your code and their code.

This can be done in a number of ways.

  • Push code into libraries, and while you do it stabilise and document the interface.
  • Institute Idioms. Idioms are easily remembered and can vastly simplify a complex implementation to a simple story from the users perspective.
  • Documentation. If your going to explain it more than once, best to capture it for future reference. Better yet if you can multimedia it with a recording of your voice/presenting it/graphs/pictures, or can link it to the source itself somehow.
  • Push back on explaining. Its not your job to describe the systems nuances to everyone who asks. If it were, you would be a technical writer not a software developer. Knowledge that was not hard won, does not stay learned. Don't waste your efforts.
  • Push implementation out of the framework up into user space. If they are seeking you out a lot to understand it, they are trying to alter it, ergo it is in flux and at the wrong shearing layer in the tech stack. Provide a plugin interface for it (such as a strategy pattern) and a default if others need it in the framework.
Related Topic