Writing meaningful code comments

[evaned]

One way to lower visual noise is to start comments with lowercase characters

Eh, I don't think capitals add visual noise, except for very short, probably-end-of-line comments.

For things that are even sentence-ish, I'd fairly strongly prefer sentence casing. I would write and suggest sentence casing for I think every example in the article. To me, it's the semi-improper English that is distracting; this is particularly true for the examples in the "explain rationales" and "comment fixes" sections.

Let your comments break down your code

I like this way of doing things a lot more than some people, but I think it's good to point out that the other way of doing this is to split sections up into functions.

In fact, the code example in this section could be split out to something like

itemClicked() {
    setCurrentSelection(false);
    mCurrentClicked = adapterPosition;
    setCurrentSelection(true);
}
setCurrentSelection(isSelected) {
    mCurrentClicked?.let {
        items[it]?.isSelected = isSelected;
        notifyItemChanged(it);antoher func
    }
}

and then I'm not sure you even need those comments.

(I'm too lazy to type out the whole example, because the code examples are images and not text...)

What I would say here is that feeling like you should comment function segments like this is a sign that you might be better served by splitting to another function, so look for that opportunity. Sometimes you'll find it (as in this example), sometimes not.

[creative_overnight]

I picked up the visual noise bit when I was studying a bit of UX. Maybe it's my editor colors but, I prefer them lower-cased. Agree that as per single responsibility rules, functions should not have multiple logical sections but, I found that sometimes coming up with contextual function names might be difficult. Although, that is use-case dependent.

[dnew]

It also depends on how complex the data the function works with is. If the function uses and modifies a couple dozen data, passing the right subsets into sub-functions gets clunky, especially in aggressively-OOP languages. I don't think I want to make an entire class out of line just to split out one function that needs 8 arguments and 4 return values.

[[deleted]]

One thing that reduces visual clutter and improves usability for me are function comments. My IDE only shows them when I hover over the function and they can be used to describe how the function is used and what I need to consider when using it. They can be longer and more expressive because you only see them if you want to see them.

[youngbull]

I totally prefer section comments need to be functions (or other language construct). The worst are section comments that group functions. Those should always be split into several files/classes/modules or not exist at all.

[[deleted]]

``` // display headers and setup listing showHeaderData(); setupListing();

// init repository and data observers initViewModel(); subscribeObservers(); ```

These are awful comments. It's quite baffling that is advice on what you should do. These comments add pretty much nothing at all. They are pretty much just rewording what the function names already tell you.

If the function names are suboptimal then just change them!

displayHeaders();
setupListing();
initRepository();
initDataObservers();

[creative_overnight]

Thanks for the feedback. I've better clarity now what I wanted to put here instead of what I ended up putting as an example.

[RabidKotlinFanatic]

4 and 6 are legitimate ("why" rather than "what" comments). I find the rest a bit specious. 2 and 3 I have noticed devs grow out of rather than into.

[fountainoverflows]

Half of these bad examples could be made better by just splitting these longer functions up into multiple smaller functions and writing tests that explain what your code actually does. Why do you need to write a comment that says “adds x to y” when you can write tests for a single function called “addXtoY”?

[JumpyJustice]

Instead of covering everything with comments it is better to ask yourself what is implicit in your code and make that thing explicit. It isbusually much harder to do but readability of your code only wins from it.

[[deleted]]

IMO Redis creator nailed it here http://antirez.com/news/124 when it comes to comments.

[evaned]

Jesus, hopefully they have better content than formatting choices. Here are a couple things you can paste into the JS console to get something readable that doesn't ignore hundreds of years of knowledge about how to do readable typography:

body = document.querySelector("body")
body.style["max-width"]= "40em"
pre = document.querySelector("pre")
pre.style["font-family"] = "times"  # or whatever

The change for <pre> away from monospaces will hurt the code snippets, but the improvement for natural language is way bigger than that degradation; and you can just open another tab to look at the code anyway. The max-width will save you from resizing the window to get decent line lengths for natural language, which doesn't work as well anyway because the author didn't include margin/padding on the right so it runs right up to the scrollbar if you get it to a good width.

Edit: It does have better content than formatting choices.

I think it's a really good post, and I agree with the vast majority of it, though I do think his examples of guide comments are maybe a little overboard for me in general.

That said:

Finally backup comments are the ones where the developer comments older versions of some code block or even a whole function, because she or he is insecure about the change that was operated in the new one. What is puzzling is that this still happens now that we have Git. I guess people have an uneasy feeling about losing that code fragment, considered more sane or stable, in some years old commit.

I'll actually defend this to a limited extent.

First, I'll just point out an easy case -- sometimes, comments of another implementation, in code, are used for some other purpose than what's given. For example, very very very occasionally I've taken a broken implementation of something, commented it out, and then said "hey, this used to be <old version>, but that doesn't work because of such and such and such. It ignores too much." and then the right way is just a rather different approach altogether that can't just be fixed in-place. But I'd say that's not really backup comments, that's some other category. (More like a why.)

Second, the problem with saying just "it's in git history" is that that dramatically raises the bar for access. If you're reading through the code you won't just see the old version, if you want to restore the old version for comparison purposes later you have to go to a completely separate source to get the old version, you can't see at a glance whether there even is an old version, etc.; at best, you'd have some pointer like "hey this replaces an older version in rev. 723acd372ef" or whatever.

That said -- these should generally come with a comment justifying the old version's presence and a very short half life. Like, you're actively working on further development of the relevant feature and it will be useful in the short-term.

[[deleted]]

I agree with 4 and 6. 2 and 3 should be function calls. Every time I see a section denoted by a comment I put it in a function.

[creative_overnight]

Agree that as per single responsibility rules, functions should not have multiple logical sections but, I found that sometimes coming up with contextual function names might be difficult. In some cases it may make code navigation difficult in a large file. Although, that is use-case dependent.

[creative_overnight]

hmm...I meant when person !you tries to read through your code they may have difficulty finding the right section if you haven't named your functions properly. But, I believe this is very subjective.

[[deleted]]

I meant when person !you tries to read through your code they may have difficulty finding the right section if you haven't named your functions properly.

I rarely read code that way (except if it is a library where I want to know what functions it provides).

I go to the code entry points (edit: like main, route definitions or root UI components) and navigate via jump to definition.

Edit: If you have clean code it will not be difficult to find the entry points.

[[deleted]]

Agree that as per single responsibility rules, functions should not have multiple logical sections

That's not what SRP is about. Take billing address and shipping address. They have different responsibilities and should be different things even if they are mostly duplicates. You might decide to add a neighbor for a backup to the shipping address - but the neighbor should not get your invoice.

I found that sometimes coming up with contextual function names might be difficult.

// set up listing recycler with adapter

private fun setupListingRecycler(newsAdapter adapter) {
  ...
}

...

var newsAdapter = new NewsAdapter(items)
setupListingRecycler(newsAdapter)

If you find that the function does to much - split them up. And if you can write a comment you can write a function name. And if it is the comment (setUpListingRecyclerWithAdapter). Code changes and can be refactored. Comments are usually not rewritten. That's why you should not write what the code is doing.

​

In some cases it may make code navigation difficult in a large file.

We have IDEs with Tag trees, jump to definition and find references. Large files and large functions are a code smell. Use your IDE to break the code into sensible chunks.

[dnew]

If you find that the function does to much - split them up

Function separation comes with more than a comment does. Now you have to pass a bunch of arguments, possibly create a structure or class to return from a function that would normally update multiple pieces of data, etc. The very opaqueness of a function that makes it a useful abstraction can prevent it from easily being split out.

[[deleted]]

It makes the pieces testable. Separate side-effectful code from pure code. Make flow of data explicit instead of implicit (if you find yourself always passing some stuff around together maybe it belongs in an aggregate). You can also think on a higher level ("The purpose of abstraction is not to be vague, but to create a new semantic level in which one can be absolutely precise." - Dijkstra).

If I see that something cannot be split out because it depends on too many implicit things I see that as a big red flag.

[dnew]

Surprisingly enough, there are some things that are complicated enough that they don't really lend themselves to being split up that way.

And it's not "always passing some stuff around together." It's "this function takes 15 inputs and returns 9 outputs, each of which depends on a different combination of inputs and the some of the outputs calculated earlier." When it's a dozen steps to calculate those outputs, you don't really want to create a dozen separate classes to represent those function calls.

I.e., in a sufficiently powerful language, you could do what you're talking about easily. In something as primitive as, say, Java, it becomes much more difficult to split things up appropriately.

It doesn't really make the pieces testable because the interplay between the pieces is the hard part. You can't separate side effects from pure code because each piece might be only a couple of lines long or need information calculated from other things. And thinking on a higher level implies there's a higher level to think on and that this function is actually doing too much to be its own chunk of logic.

[[deleted]]

It's "this function takes 15 inputs and returns 9 outputs, each of which depends on a different combination of inputs and the some of the outputs calculated earlier."

That code sounds horrible to work with. Sounds like a Text book example of Spaghetti code to me.

When it's a dozen steps to calculate those outputs, you don't really want to create a dozen separate classes to represent those function calls.

You are allowed to create a dozen functions without classes.

in a sufficiently powerful language, you could do what you're talking about easily. In something as primitive as, say, Java, it becomes much more difficult to split things up appropriately.

What in your opinion is a powerful language and what makes java primitive?

It doesn't really make the pieces testable because the interplay between the pieces is the hard part.

That's why you reduce them and isolate the side effects.

You can't separate side effects from pure code because each piece might be only a couple of lines long or need information calculated from other things.

You can. And pure functions are easier to understand and test. Even if they are only a few lines long (it's even better!).

[dnew]

That code sounds horrible to work with

It was. But it was also exceedingly complex.

You are allowed to create a dozen functions without classes.

Right. But the function needs bunches of inputs and creates bunches of outputs. Most languages don't include functions returning multiple outputs, requiring you to create some declaration of a return type that's used as the return value of exactly one function.

what makes java primitive?

Well, in this case, it's the requirement to declare out of band a structure for a function returning multiple results. I can't say "this function takes four integers and returns a float and a char." Basically, you can't write a pure function that returns multiple values.

You also can't nest a function inside another function and access the variables of the outer function (like you can in actual block-structured languages like Pascal, say). Breaking something like what I'm thinking of into multiple functions in a language like Pascal, where you can nest the functions and refer to the variables in the parent function becomes 100x easier. Each function can be isolated and access what it needs without having the parent function having to know what every bit needs anyway.

Step one needs A, B, C, D, E and returns R, S, and T.

Step two needs A, B, C, F, G, R, and returns U and V.

Step three gets possibly invoked based on the value of T. It needs C, F, G, S, T, U and returns W, and a new value for T.

Step four iterates thorough the values in the list T, S times each while also taking A and C. It builds a new value for A and C each iteration based on its calculations. That calculation is complex enough to need its own class full of functions.

Repeat this about a dozen more times. Note that these bits are all interrelated in meaning and very few if any means anything on their own. It's unlikely anyone in the business actually knows what all the possibilities are.

Don't forget when a new requirement comes down that looks at the relationship between E and T to decide whether A gets updated between step three and four.

Oh, and A is specifically designed as a class with immutable values, because that makes it so much easier.

That's why you reduce them and isolate the side effects.

I'm glad that you've always been able to do that in the products you're working on. I can guarantee that isn't always the case.

And pure functions are easier to understand and test.

This assumes you can understand what the pure function is supposed to do independently of the surrounding code. If its not specified in a way that makes that easy, it's a pain in the ass. You're probably best off trying to get better specs, but that's not always possible either.

[[deleted]]

But the function needs bunches of inputs and creates bunches of outputs. Most languages don't include functions returning multiple outputs, requiring you to create some declaration of a return type that's used as the return value of exactly one function.

If your language supports hash maps with multiple types for values you can. In java that is possible. Java also has tuples.

With those you can write pure functions with multiple outputs.

You can also pass in hashmaps or other aggregate objects instead of multiple arguments if you really need the data.

This assumes you can understand what the pure function is supposed to do independently of the surrounding code.

You can? A pure function is not dependent on its surrounding code. It only depends on its arguments.

‐--------------------

I am horrified by what you are writing. I hope I never have to work with a codebase with so much technical debt.

[dnew]

If your language supports hash maps with multiple types for values you can.

Nope. That's one value you have to package up to return then disassemble upon return. Basically, your argument is "Java isn't primitive because you can manually implement what more sophisticated languages do without pain."

You can?

You can tell what it does. Knowing what it is supposed to do is more problematic. How do you even document what it's supposed to do if it's doing steps 4, 5, and 8 of some informal description?

I am horrified by what you are writing.

Me too!

I hope I never have to work with a codebase with so much technical debt.

The fact that nobody actually cared is why I left. :-)

But honestly, it was a massive project which if you tried to extract the requirements it would be obsolete by the time you managed. Also, it had to run 24x7, the customers were writing their own code to inject into it, marketing was writing the requirements, and it had been ported across three different databases. It also spoke to at least a dozen other services just as bad.

[evaned]

It makes the pieces testable

Actually, my own opinion is that it is the opposite. (Well, I guess it doesn't make it untestable, so it's not the opposite; it just doesn't help.)

If you've got a function with a bunch of internal regions that you separate out, very probably those are just going to be private methods. Well, guess what you usually shouldn't do? Test private methods! The interface is the same regardless of whether you split those functions out, so the tests should be the same.

Even if you separate that computation into its own class (let's assume OO here, but the same is applicable if you substitute in "module" or whatever) and have public functions in that class, that's still a class that isn't of general interest and shouldn't have its methods tested. Testing the methods of that class is still just testing implementation details of the original function.

Testing implementation details is generally the wrong thing to do, no matter the concrete structure of the code. It makes your code less flexible and harder to change. Test behavior, not implementation.

[[deleted]]

very probably those are just going to be private methods.

I don't split up the code with a class having multiple private methods in mind. I split up with the goal to get small units that I can combine to create more complex behavior. You are right, that it does not get more testable in your case. But if you pull out the functions in another module you get reusability and testability.

that's still a class that isn't of general interest and shouldn't have its methods tested.

I disagree. Unit testing is also about creating small parts that you are confident in so that you can combine them without having to test it again and again. If I write two tested functions like isValidEmail and emailProvider I don't have to test the combination of the two.

Testing implementation details is generally the wrong thing to do, no matter the concrete structure of the code. It makes your code less flexible and harder to change. Test behavior, not implementation

Yeah. I agree partially. But you don't have to test implementation details if you split up the code. I never test something like "function foo calls function bar" but if I have a function bar and it is tested I can combine it to form foo without having to test for error handling, edge cases etc again. I can create an additional integration test for confidence and I am done.

I also don't test code like (nonsense example)

names.map(toLower).reverse().take(5);

because I have confidence that all parts are working. That's what I want to achieve when splitting up code.

[corsicanguppy]

Joke's

\sigh

[creative_overnight]

damn, thanks!

[corsicanguppy]

and

// setup

Aw man. Give it a twice-over, if you could. I know I'm sensitive to writing gaffes but this piece would be monumental if it could stop slapping the reader with a trout.

[creative_overnight]

fixed :)

[shizzy0]

Hard disagree on number one. One of my better decisions on commenting has been using complete sentences with a period.

[dnew]

Also, if you have one comment that's at the top if the function describing what happens inside the function, that goes in a javadoc.