Alexey Chernyshov

In search of perfect code

Communication as a Code

Up to this time, I mainly wrote about communication and structure in the blog. I might be a perfectionist and sometimes too picky, but I believe clear communication is the thing in software development. While it is essential to have good architecture and write robust code, it’s also very important to have the code itself and surrounding technologies clean and clear. Almost every piece of code will be fixed, rewritten, or refactored at some point, and when it happens, I would rather have it understandable. It is almost the same as the idea to have the code base and structure that is easy to modify (because everything might be changed at some point): if it’s not easy to process, it’s not easy to work with.

I saw many things that I would’ve liked to be done differently to be transparent. I’m going to share a couple of examples and express my thoughts on them.

Comments

Oh, I used to hate comments in general because I felt they were almost always useless. I thought the code had to be self-explanatory, and my stance was to prefer making the code clear instead of writing a comment. The only type of comment I could tolerate was a comment explaining a decision behind a piece of code. That world was so perfect, I miss it sometimes.

But for some reason, I’ve changed my opinion recently. I started to see a lot of places where a decent comment might have saved some time. Let’s face it: we can’t obviously make all the code we write and fix ideal. Every time we have to think of a middle ground between perfect code, business tasks, and other considerations. Thus, a lot of times, when I see some weird workaround or hack obviously trying to handle a specific case, I’d like to know why it was written. Of course, it sounds a lot like a comment explaining the logic behind a decision, which I’ve written about a couple of sentences ago, but now I think it is appropriate in a lot more places than I thought before. Maybe that’s because I’ve introduced myself to some code that I’d rather not see, but whatever.

However, this doesn’t change my disliking a lot of other comments. Here’s an example:

// GetMyNumber
// returns My Number 
int GetMyNumber() const
{
  return _MyNumber;
}
// SetMyNumber
// Sets My Number to the specified value
void SetMyNumber(const int number)
{
  _MyNumber = number;
}

This is just noise. These comments are absolutely useless and even harmful since they limit the number of information a screen can fit. There’s no way they can be useful for anyone except for the author, who might be proud of themselves, stating they describe every function they write. If for some reason your IDE bothers you with ‘Don’t you forget to add a comment to every publicly exported artifact’, ditch it for vim.

Next:

auto rc = CallMyFunction(...);
// handle error
if (rc == 0)
{
  HandleError(rc);
  return;
}

That doesn’t help much either. I believe everyone can see that’s error handling. Don’t waste time and space stating obvious things.

// Last modified by John Ivanov
// History:
// Jan 12, 2013: file created
// Jan 31, 2013: fixed a buffer overflow bug (MP-104882)
// Sep 07, 2014: function `MyFunc` refactored and unit tests are written

Version control does it for you. Don’t bother tracking yourself.

Okay! Comments I can tolerate and even like sometimes are the comments explaining details or decisions. Sometimes it is useful to put a relevant piece of documentation into a comment:

char buf[n];
int rc = snprintf(buffer, n, "something happened: %s", mystring);
// snprintf returns the number of required characters, 
// not actually written characters
if (rc > n + 1)
{
  a larger buffer handling here
}

Commit messages

That’s simple, right? Just put what comes to mind first and commit. And push. And forget. I’ve seen some ridiculous commit messages lately. How do you think what kind of changes does a comment named ‘+’ bring? And what about ‘build fix’? Or my favorite ‘fixed pull request comments’? I wouldn’t like anyone writing such messages to get into a situation where their lives depend on finding a particular change quickly. I mean, I hope none of us end up in such circumstances, but still. The smartass in me would like to go with the standard dull story that every commit moves code base or product forward, and that’s what important, blah-blah-blah. But the truth is: communication is what matters here. Whenever someone looks through a commit history, they are definitely not killing their time but looking for something. And if you’d like to help that person, when committing, ask yourself, ‘what is this change for?’ or ‘what exactly I changed?’ instead of ‘why I did it?’ and ‘what is this for?’ The latter two make you feel better (because of purpose), but your Karma is cleaner with the first one. It’s better for Karma’s sake to choose ‘OpenSSL is now explicitly linked’ over ‘build fixed’ and squash your ‘fix pull request comments’ commits instead of leaving them.

Bug description

This one is for those who fill tickets in whatever tracker. Sometimes people think that only the steps to reproduce are essential, and then the person following them can find out everything else themselves. But there are a lot of cases when text description matters and someone would like to understand what the ticket is about, but might have no time or right equipment, or the bug might be unstable.

Thus, document observations first, then proceed with conclusions if you’d like.

Example:

Steps to reproduce:
1. Open Firefox and navigate to mysite.com
2. Fill in credentials (test@mysite.com / mypassword)
3. Click 'Log in' button
 
Actual behavior:
Login doesn't work.

Expected behavior:
Login works.

Can anyone deduce what’s happened? Did the server respond with 401 or 500? Does it show a message that login has failed?

Sometimes additional artifacts like an attached picture or video might help, but let’s not count on them much; the description should be sufficient to get a decent understanding. Let’s change it to magically remove the mist of doubt:

Actual behavior:
'Log in' button does nothing. The browser stays on the same page,
it doesn't try to load anything.

Expected behavior:
After clicking the button, the browser proceeds with authentication
and shows my personal page.

Other stuff

A lot of times, developers tend not to care about things that don’t matter. Or at least the ones that seem not to matter. For example, why bother carefully designing HTTP response statuses if no one really sees them? If you have a REST endpoint that checks if a well-known file was changed, don’t bother thinking about the response. Feel free to respond with a 404 if a file wasn’t found for some reason! You’ll see how surprised your colleagues will be when they come to you and ask what is wrong with their request because the server responds with 404. You will tell every one of them that this is a smart way to report an error. And waste everyone’s time because the majority of people expect 404 to mean ‘nothing exists at the URI’, and not some gimmicky response.

Don’t do that. Developers across the globe agreed on using specific HTTP codes to communicate particular messages. If you’re using them for other purposes, think twice. Same applies to almost every standard out there, including design patterns.

And the last one. Log messages. When a developer implements a feature, they are in the zone, they know every nuance, and they have no doubts their log messages are clear and understandable. But it is worth ensuring that the same statements could be read and understood in two years by a colleague who doesn’t know every detail of the feature implementation. Basically, every software developer familiar with the area and the software should roughly deduce what is happening by solely looking at the logs (without looking at the code). And log messages is the place where details matter. ‘File not found’, for example, doesn’t help while ‘Can’t load database (C:\ProgramData\MySoft\my.sqlite): File not found’ is very good.