#begin

Today we are going to look at the next chapter of the Clean Code book by Robert C. Martin. In the previous blogs of this series we discussed the first two chapters of the book.

So let’s dive straight in and continue with the fourth chapter in the book – Comments. On the first page of this chapter there is a very nice quote that sums it up really nicely. It says, and I quote: “Don’t comment bad code, rewrite it”. Haha. I mean, how often have you seen crappy code that is explained by even crappier comments that are outdated and flat-out wrong. So, in this chapter we are going to take a look at all the advice Uncle Bob gives related to comments in code. There are many kinds of comments that are really bad or unnecessary but there are also kinds of comments that really do justice to their existence. Let’s dive in and see what he has to say, shall we?

Comments

So Uncle bob starts off by setting things straight immediately. He says that nothing can be quite as helpful as a well-placed comment, but on the other hand, nothing can clutter up a file more than dogmatic comments. And nothing can be quite as damaging as an old comment that propagates lies and misinformation. He says that comments are at best “a necessary evil”.

And I think I agree with him. I really dislike comments in code if they are not needed. I’ve seen lots of code that has like a line of comment, and a line of code, then again a line of comment and a line of code. So every line of code is covered by a comment. That’s just so useless and I know that some teachers in university might ask for students to do this but this is really horrible! Please stop doing this because this is most definitely NOT industry standard nor is it best practice!

Uncle bob says that every comment is in fact a failure our ability to express ourselves in code. What he means is that sometimes the code you write is so cryptic and verbose that the only way you can explain what is going on is by adding some comments. So what you should do in this case is try and refactor the code so you can instead express your ideas in code and no longer need to comments. So comments do not make up for bad code. Always to to explain yourself in code instead of comments. So if you have for example an if-statement that checks a condition like for example: if(player.speed > 0 && player.direction = Vector3.Left) you should extract out that condition into the player class for example and name it something like IsMovingLeft. So now, when you want to check for it you can simple call that in your if-statement so it reads like if(player.IsMovingLeft) which is far better and more intention revealing. If you do it like this, you probably do not need to add a comment above the if-statement that says: “the code below is checking if the player is moving left”.

But before we burn all the bad practices around comments, let’s talk about what kinds of command are acceptable and generally good to add to your source code. But always remember that the best comment there is, is the comment you did not have to write!

The first obvious example Uncle Bob gives are those darn massive legal comments you will find on top of many files in many repo’s. Due to the corporate coding standards there are sometimes required for legal reasons. Such a shame that we need to go as far as this to add this nonsense in every single file. But nonetheless, we need them for legal reasons. Luckily, many IDE’s will automatically collapse these legal comments on top of files. Thank god for that, or well, JetBrains for me that is! You hear that JetBrains, you are god! Haha. So yeah we cannot simply remove these comments because they are required to protect the business so we have to live with it.

The next acceptable comment is a simple informative comment. The perfect example of this is the explanation of a regular expression, and telling what it does. I’ve done this personally many times as well because no sensible human is able to remember the regex syntax for longer than 24 hours. I’ve even gone as far as pasting in the stackoverflow url where I found that damn thing in the comment. Because stackoverflow is where all regexes are born. I’s damn ugly but when you have some very complex regex it’s very important to have some information about it. What I also like to do is to cover the regex logic with a shit ton of unit tests to try and express what the regex does by mere testing. So other programmers can look at the unit-tests and see what it is supposed to do. But just remember, a simple comment that explains the regex is often not a bad idea.

Another type of comment that is generally accepted is the comment that tries to explain the intend of the code. Sometimes just a mere informative comment is not enough and you need to add some rationale. An explanation why the author of the code made this particular choice. A nice example for this would let’s say adding some comment to explain a unit test. I personally, don’t use fancy mocking frameworks for testing for example although you sometimes need to write testing mocks. Mocks are stubbed out classes that return predictable values for the sake of testing. So, in some cases I’ll add some intention revealing comments to these mock classes to better explain why I return or implement things like they are in there. For example, in some particular test case I need to test certain things of behaviour. So I create a mock for and only implement one function, in the rest of the functions I simply throw a NotImplementedException because I don’t touch these functions in this particular test scenario. So I’ll add some information in the summary of the function to indicate that these functions are not supposed to be called, and if they were, my test would fail because an exception will be thrown.

So by adding these comments the chance that there is confusion about this is reduced.

Let’s take a look at another type of comment that is deemed good; sometimes it is helpful to translate the meaning of some obscure argument or return value into something that’s readable. And generally, it’s better to simply refactor and name things properly but when it’s in code of some standard library or code you cannot change you could add a comment that clarifies things. I’m trying to come up with an example of the Unity3D API, because I know for sure there are a couple of things in there that I always found confusing or simply really badly named but I can’t think of something. But, I’m sure there are things that have obscure naming. So, let me know if there’s something in the API that always confuses you. Well maybe, a stupid example is the WWW routines to send HTTP requests with are darn confusing because, you are recommended to also use them for accessing the file system. And are required even to use them if you need to get files from the streaming assets on android since you cannot simply access them through the System.IO.File class because they are compiled into the APK or OBB file. I always thought this was particularly confusing since I like to use the default File or stream API’s for this provided by the System.IO librabry, using some HTTP workaround hack for this just seems dirty to me but it is what it is.

Next there is the type of comment that warns for consequences and I sometimes add comments like this in code, or in function summaries that are not thread safe for example. Especially in a Unity3D context, multi-threading can be a difficult concept to start with since most of the Unity3D API is not thread-safe. So, when I’m writing some heavy concurrent program but I need to do so potentially dangerous things I’ll add a comment that says that the code you are about to run is not thread-safe and thus you must dispatch it to the main thread and then continue on an off-thread if you need to. If you are not familiar with the Dispatch pattern you should google it for a second. It’s a really nice pattern that can sometimes help you out of difficult concurrent situations. The pattern boils down to this: If you are running some code in an off-thread and you need to call some things from the Unity3D API, which is only accessible on the main-thread, you dispatch a function, like a lambda wrapped into an Action object to the Dispatcher. This dispatches simply implements a concurrent queue and executes the actions within the update loop in unity, and thus it is back on the main-thread so you can access the Unity API side of things again. It’s really useful in some tricky situations. But I also want to give you some warning with this particular pattern and that is that if you use this dispatcher pattern and you find that you have bugs in you system, it is very difficult to debug this because your stack-traces will begin in the Dispatcher update look, and not in the actual code that started it. So, just keep that in mind. The same also happens with coroutines sometimes.

But damn, I got lost a bit here. We were talking about comments that explain consequences. So I said I sometimes add these in cases where I warn people of things that might happen. The example Uncle Bob gives in the book is nice as well. He has an example in here of a comment that warns of some test case that will take a very long time to run. It’s a test that generates a really large text file and then tests certain conditions from it. This is very welcome as well I guess. So the comments warns the developer to not run the test during development of some feature, but do run it before checking in the code or during regression testing for example.

The next kind of comment Uncle Bob says sometimes can be good are ToDo comments in code. And, I disagree with him on this one. I personally think that ToDo’s are always bad and should always be removed since a ToDo most likely turns into a Don’t do. I bet you know of some ToDo’s in your code-base that have been there for years! So what I do is check who’s the author of the ToDo comment, send him or her the code snippet with the comment and tell her to create a ticket in jira of whichever system you are using. Then I delete it. Period. ToDo’s have no place in code, unless is is to remind yourself you need to do something, but once you merge your code into the development or master branch all ToDo’s should be removed. And if there are things that still need to be done, Please, just make tickets of these things so they are properly tracked. This way everyone can see that some things still need to be done and they can be planned and accounted for. So try to keep your code ToDo free, we have project planning and tracking software for a reason!

Now there are only two kinds of good comments left and the first one is a comment that amplifies the importance of something that may otherwise seem inconsequential. The example in the book is about explaining why a particular string should be trimmed from spaces because else it would be a different list. Think for example you need to convert some JSON data or CSV to a dictionary or hashtable. You must be sure that keys in the dictionary are correct and thus you trim them and maybe, cast them to lower or upper for example. You might just want to add a simple comment in there that explains why you execute that logic.

The last type of comment that Uncle Bob deems to be acceptable are comments that are used to generate documentation. In C# these would be the summaries you can add to classes, functions and fields or properties. You can use this to explain things. So when you run your code to something like doxygen some nice documentation is generated. Just remember to not add summaries to like an integer called duration, and make the summary say “the duration”. This is useless and doxygen is intelligent enough to generate the exact same summary for you automatically. So don’t make summary comments be a copy of the thing they summarize. I bet you have seen this before 😉

So, now we have described some of the good types of comments, we are going to dive into the comments that are simply bad and should always be removed from code. No excuses! I’ll try not to sound too pessimistic when talking about them, haha.

He starts off with a type of comment he says is just mumbling and the example he gives is some code loading some properties files that is covered by a try-catch block but the catch block does not contain any executable code yet just a simple comment that says “no properties files means the defaults are loaded”. But from reading the code in the try block we cannot determine whether the defaults are actually loaded because that functionality is probably in some other function somewhere else. So the developer who wrote this tried to be helpful but made matters worse since we have no idea where the defaults are supposed to be loaded. This is what Uncle Bob means by mumbling, just placing seemingly random comments around the code that just confuse people.

The next type of comment is the redundant comment and I think we glanced this one a bit earlier. What he means by this type of comment is the comment that simply restates the code that it is commenting. Like having a integer called duration and then having a comment or summary above it that says “The duration” or a simple comment above a constructor that says “constructor”. It almost feels offending doesn’t it? I mean, we are programmers and we know how to read and write code so adding these kinds of comments is totally unnecessary. They don’t need to add comments saying that something is a constructor! We are well educated and we will be able to figure it out, right?

But a worse comment would be a misleading comment. I mean we have probably all seen misleading comments, right. These would be comments that state something about the code, but the code does the complete opposite, or simply something totally different. But these comments can sometime creep into the code unknowingly. Image some comment describing an algorithm. The comment maybe describes some function calls and variables or fields. Now, when we refactor the code, we shuffle things, rename things, or move things to new classes. However, the act of refactoring does not edit the comments, and we programmers will most likely forget to edit the comment that described the algorithm. So after the refactoring the comment is misleading and probably a lie. So, the best thing to do now is delete the comment in my opinion. I mean it’s worthless and misleading now anyway.

The next type of comment is a funny one I think and these are, mandated comments. When comments are mandated by some figure who wants them in, you will get many redundant comments all over the codebase. I mean, if you are obligated to add summaries all throughout the code, you will run into the situation where you will put a comment that says “constructor” above the constructor of each class. Just to comply to these mandates. And, I’ve seen this in code before, I’ve done this exact thing and trust me, the code won’t look any much prettier. All your files will grow in size a lot because if you add summaries above everything that is public your fields will gain at least 3 lines and functions will probably gain more because of the documentation you add for each argument. The same goes for those comments that have the name of the developer who wrote the darn class. I mean, we all have source code control systems and we can very easily find out who wrote the code you are looking at. We don’t need those worthless comments that state the original developer’s name. Would you add your own name to the list if you edit the code? That would make the comment infinitely long.

And this is a nice segway into the next type of comments that Uncle bob says are bad and those are actual journal comments. I don’t think I have ever seen them in a project but I can see why they used to be useful. Back in the days when we did not have source code control systems. I mean, Uncle Bob has been going at it for such a long time that he’s seen it all. So these journal comments are comparable with like, commit messages in git. But back in the day there was no git and thus they used another way to communicate changes that the code underwent. But nowadays we don’t need this clutter in the code since we have systems like Git that track all this stuff for us. So if you see these kinds of comments, just remove them since they are in git anyway.

The next type of comment that Uncle Bob describes are called noise comments, these are all related to what I have discussed previously for like 2 times now already. But noise comments are these comments that state the same as the thing they describe. Like a comment that says “time of day” above a variable or field called TimeOfDay. These are noise.. just remove the comments, we know what these variables mean. And another thing, what sometimes happens with these kinds of noise or mandated comments is that they are copy pasted around. This will sometimes lead to variables, fields, functions or classes having incorrect comments. Since they were copy pasted but not changed.

And next is a comment that I think we described earlier as well. Uncle bob tells you that you should not write a comment for code, that you can perfectly extract to a new function. As I said before, always give it your best effort to express yourself in code. Try to make the code as readable as you can so you do not need comments to describe what you mean. Your code must be intention revealing. And this can be hard some times but if you put in that extra effort your code will be far superior to code that has not been considered heavily for cleanliness.

The next type of comment I see pretty regular, but are not really needed since we have a nice feature in C# that handles this. I’m talking about comments that are used as position markers. Position markers are these comments that say like: Fields, and then all the fields follow, then a comment that says, Function and all the functions follow. So the comments place markers in the source file where certain code resides.

But in C# we have things called regions. If you really want to use these kinds of markers, wrap the code in regions if you like. Just make sure that, by adding these regions you do not make the class too big and have to many responsibilities. A common problem with classes that have regions is that they do not comply to the S in SOLID, the single responsibility principle. The single responsibility principle says that: A class should have only one reason to change. Which may sound strange considering that the principle says nothing about change. But what Uncle Bob meant with the single responsibility principle is that it should only have responsibilities to one stakeholder. A simple example would be a class that talks both to the web, and local database. The class has two reasons to change now, when something on the web-side of things changes, it needs to change, if something in the database changes, it needs to change as well. But, enough about the single responsibility principle. The SOLID acronym is worthy of it’s own podcast and if you are really interested at this point, I wrote a blog series about SOLID some time ago which I will post in the show notes for you to read if you want to.

Next there is a very unusual and actually funny comment type if you ask me and those are closing bracket comments. If I never read this book, I would not have thought this was a thing. So closing bracket comments are comments that mark the end of an if, while, for, foreach or try-catch for example. So you would add a comment that says “if” after the closing bracket of the if statement. I mean, wow, that’s amazing right? I guess this is really oldschool. I’ve never seen this in any code-base I have worked on. Have you seen this? If so, please let me know I’m pretty curious.

Then there is the mother of all bad comments and that is, commented out code. I agree with Uncle Bob on this one, and it is the worst kind of comment you can ever find in a production system. And why? Well, commented out code can always be removed, it will probably only spread confusion and clutter up source files. But people will say, well I might need that code in the future. And then I tell them it’s in git and delete it. When I see commented code in a file, I don’t read it, I just delete it as soon as I can. I mean, it’s not being run at the moment and thus has no value, and if you need it search it in git. Another aspect of why commented code is bad, is that, when you refactor the code, you will probably not refactor the code in the comment, and thus is is broken anyway. I mean, when I use automated refactorings like extract function, extract class, extract interface or rename I’m not exactly sure what code is being touched in the code-base. I trust the tooling that it works and run my tests if there are any. I’ll see the changes in git when I check my code in, but if there’s commented code around my refactored code, I’ll remove it too before I commit and check it in. So commented out code is an abomination you should destroy asap to keep your code clean.

Now, do I ever comment code? Yes, of course I do, but I will never merge the commented code into the develop or master branch. So in your working branch, which is for you personally, you can comment code and keep it there for the time being. But when you check the code in and merge it you should remove the commented code. So again, when you see commented code, delete it, it’s in Git anyway.

So next are comments that contain HTML code for generation purposes. Please, don’t do this, these comments are worthless since they are darn unreadable in you code. And I guess that some companies have policies around this and I think these policies have to change! But policies can’t be changed you might say. My response would be that policies are written by human beings, human beings have the capacity to change, and thus policies can as well. But, I get that sometimes they won’t and you are stuck with these horrible HTML comments. I’m not sure if there’s like IDE features or plugins that will draw the HTML stuff in different colors, and highlight the important information for the programmer. If there are none, than, hey plugin writers, please give us some support for this!

And the last comment that Uncle bob describes are comments that provide far to much information. And trust me I’ve seen these before where like the entire Wikipedia page was pasted into the code to describe an AES encryption algorithm. Please.. I mean, remove it and just say an AES algorithm is used. You can probably find out just be reading the class name. If I need information about AES encryption, I will google it myself and see what the heck is going on. I think many of you will agree with me on this and if not, let me know I really would like to know why you would prefer these kinds of comments.

So we have finally covered the fourth chapter of the clean code book. And I know this long a discussion dedicated to comments may have been a bit boring but it is really valuable information. I mean, comments can be a big form of clutter and misinformation in your code and will impact the cleanliness. So I hope I presented the information in nice manner.

So that’s it for the fourth part of this blog. The next blog will cover the fifth chapter of the book called Formatting. If you have any comments about this blog, please leave them in the comment section. Puns intended!

Thanks for reading, and smell you next time!

#end

01010010 01110101 01100010 01100101 01101110.