4 ms·

The tests say what the code is supposed to do. And, unlike the comments, they cannot be out of sync with the code because they will fail.

jbay808 6y ago
Ok, so for example, let's say I have a function npow2(x) that returns the next power of two above x, unless x is a power of 2 already, in which case it returns x. And I want to use this to get the next power of 2 below x. I can write: y = npow2(x+1)/2; //get y such that the next power of 2 above y is at least x. The comment says exactly what the code does, but at least it also says that that's what it's intended to do. This lets a reviewer check correctness, add optimizations, and also understand the intentions at a glance. The why is important too, of course. Now I could alternatively add unit tests that specify a large number of input cases in which the output happens to be the previous power of 2. But unless I make the test cases exhaustive, I'm only hinting at what this code should do, not defining or explaining it. I could break this out into a function too, called prevpow2(x), which might or might not be appropriate in this case. But if so, I'd expect that function to have a docstring explaining... exactly the same thing as the above inline comment: what the function supposedly does.
- tigershark 6y ago
  Adding a comment there is not useful, it should be added only in the function definition if needed. In this case it’s needed because the function does something unexpected. I would call it nextPower2(n) rather than npow2 and add a comment in the definition saying that the function returns n if n is a power of 2 because it’s unexpected and it can’t be inferred looking at the function name. As I said before, comments for unexpected behaviours are not only perfectly fine but extremely important. Also removing all the useless comments will increase the relative importance of the ones remaining since you will have only comments for really important stuff. Edit: And anyway in the tests you should have at least these two test cases: Test nextPower2 returns n if it’s a power of 2 Test nextPower2 returns the next power of 2 if n is not a power of 2
  - jbay808 6y ago
    The comments are informing the reader about the trick that turns npow2 into prevpow2. Your reply seems focused on npow2 itself, not the line npow2(x+1)/2 that I said deserves a comment. I agree that npow2 should have the tests and documentation that you say, but in my example I'm taking that function as given, eg. by a library.

jbay808 6y ago

Ok, so for example, let's say I have a function npow2(x) that returns the next power of two above x, unless x is a power of 2 already, in which case it returns x. And I want to use this to get the next power of 2 below x. I can write: y = npow2(x+1)/2; //get y such that the next power of 2 above y is at least x. The comment says exactly what the code does, but at least it also says that that's what it's intended to do. This lets a reviewer check correctness, add optimizations, and also understand the intentions at a glance. The why is important too, of course. Now I could alternatively add unit tests that specify a large number of input cases in which the output happens to be the previous power of 2. But unless I make the test cases exhaustive, I'm only hinting at what this code should do, not defining or explaining it. I could break this out into a function too, called prevpow2(x), which might or might not be appropriate in this case. But if so, I'd expect that function to have a docstring explaining... exactly the same thing as the above inline comment: what the function supposedly does.

tigershark 6y ago
Adding a comment there is not useful, it should be added only in the function definition if needed. In this case it’s needed because the function does something unexpected. I would call it nextPower2(n) rather than npow2 and add a comment in the definition saying that the function returns n if n is a power of 2 because it’s unexpected and it can’t be inferred looking at the function name. As I said before, comments for unexpected behaviours are not only perfectly fine but extremely important. Also removing all the useless comments will increase the relative importance of the ones remaining since you will have only comments for really important stuff. Edit: And anyway in the tests you should have at least these two test cases: Test nextPower2 returns n if it’s a power of 2 Test nextPower2 returns the next power of 2 if n is not a power of 2
- jbay808 6y ago
  The comments are informing the reader about the trick that turns npow2 into prevpow2. Your reply seems focused on npow2 itself, not the line npow2(x+1)/2 that I said deserves a comment. I agree that npow2 should have the tests and documentation that you say, but in my example I'm taking that function as given, eg. by a library.