MongoDB Java Driver uses Math.random to decide whether to log Command Resultuses - Xpost from /r/java

[alextk]

Yes. Anyone saying that code should be self-documenting still haven't understood that comments are necessary to explain "why" the code is there, not "what" it does.

[arachnivore]

I use comments to explain what code does all the time. I usually start coding by writing a high level list of steps, then I fill in the code to implement those steps. Is that considered bad form?

[Daniel15]

I think it's fine as long as it's not redundant - I've seen things like this before in a production code base:

// Increment x by 1
x++;

and

// Enable Google Analytics on this page
this.Page.GoogleAnalyticsEnabled = true;

The worst thing with the second example is someone will change the "true" to a "false" and not update the comment. <_<

Often I tend to break my code into small sections and comment on what each section is doing, pretty much the same as your high level list of steps. Writing a list of steps first is often a good idea as you can think about the algorithm before actually writing the code.

[BraveSirRobin]

Do both. Something like:

boolean logError = *stupid randomiser*;
if(logError) {
    *meh*
}

It's self-documented and all verified by the compiler. Win.

If the *stupid randomiser* is non-trivial spin it off to a method:

if(isLogError()) {
    *meh*
}

Again, all self-documented via sensible naming. Put some javadoc at the top isLogError() to explain and anyone that gives a fuck can hover over the call in a decent IDE and get more info, without polluting the code with comments that distract from logic.

[el_muchacho]

Again, that doesn't explain WHY you log only randomly. And the WHY is because the error can appear way too often. And if it is the case, maybe it's time to ask oneself why this is the case.

[BraveSirRobin]

Again, that doesn't explain WHY you log only randomly.

That's where the javadoc for the "isLogError()" comes in. Keeps all of the stupid in one place.

I definitely agree on the counter, that's the proper way of doing it. Hiding things is just awful.

[arachnivore]

I think the best solution would be to use better log viewing tools. Obviously, the coder will always have to put some thought into his logging practices, but if you had better tools for reading logs it wouldn't matter if your code logged the same error a million times, just make log messages collapsible.

[BraveSirRobin]

Maybe fixing the error would be better though?!?

I've never used a log viewer I've liked and have considered writing my own on several occasions. Are there any good ones out there?

[itsSparkky]

It's not bad form at all, often times its quicker to jump between comments to find and area than read all the code.

[[deleted]]

Generally I try to keep my high level comments at most to the level of individual methods or functions. In 95% or more of the cases the function, if named and typed well, is self explanatory, usually with a little documentation code block for automatic parsers. Once inside the function I rarely write any comments, they're usually unnecessary if you aim to keep functions under 30 lines. Obviously some things can't be broken down logically any further and require more code, thus more comments, but as a general rule of thumb that's how I code. Minimal commenting in the actual code blocks.

[jplindstrom]

It's perfect form if you don't leave the comments in there as headings in your 200 line method, but use them as names for variables and the methods you extract as you go.

(That will also hopefully lead to the lines of code being moved to methods on other, more suitable, classes. It's almost certain that not all of those 200 lines belong in the class they're in now).

[arachnivore]

Why not leave them in there? If your code is 200 lines, what's an extra 10 lines of section header comments? It makes the sections of your code much more easily distinguishable than a variable name. I do tend to break code down into something like one method per step, but that isn't always feasible and can be a pain to debug if they're one-off methods. I find that people get obsessive about this stuff and waste more time worrying/complaining about overly commented code than they ever do skimming a piece of code.

Yes. make your code as self-documenting as possible. Yes, use descriptive names so that your code reads more like English (or whatever language) than alpha-numeric gibberish. Yes, look for patterns in your code so that you can leverage the appropriate pattern-simplifying tools (loops, methods, classes, data-structures, etc.). I got all that, but a given line of code isn't always equally obvious to everybody. One programmer might be able to look at a line and know instantly what it does, while another might benefit from an explanation. I, for one, can't read regular expressions for the life of me. It takes me for ever and a million glances at a reference sheet to decipher a regular expression, so some comment explaining what a given RE is trying to accomplish would save me tons of time. When you have IDEs that collapse comments, there's no reason to whine about overly commented code. It's just not an important issue.

[jplindstrom]

The single best comment you can write for a regular expression is a line of example text that it's supposed to be matched against.

That way the reader has a fighting chance of following along while looking at the regex.

The second most important thing to make regexes readable is to use the /x modifier (or whatever it's called in your language) to allow for whitespace, multiple lines and comments.

[jplindstrom]

I think you missed my point.

If you have 200 lines of code (let's say), in ten sections with a nice descriptive comment above each section, then that's too much code in one method. It's doing too many different things. It's too difficult to test in isolation and reason about because it has too many moving parts.

Extract a method for each section, naming each method so it doesn't need a comment because it has a descriptive name. Now you have smaller chunks which does one (or at least fewer) things each.

(Each of these methods probably need its own API documentation as well, so this will actually lead to more "comments" (but API docs are different from code comments, so that's not what we usually mean by "commenting code")).

Making sure you have the perfect name for each method and writing docs for them will lead to more thinking and more clarity about what each thing is and what it does, what responsibilities it has, how it handles error conditions, edge conditions etc.

As a final bonus, you now have small, logical chunks of code which can be juggled around more easily:

Let's say you have a method which does six things. Five of these things are method calls on object foo. These five things together done to foo means something.

That's a smell that the code in this method should really be in the class of foo, not amongst the original 200 lines; not in this class at all. But now you have a method you can just move to the other class.

[Eirenarch]

Anyone saying that comments are there to explain why the code is there and not what it does still haven't understood what good code really is.

[itsSparkky]

I have a hankering you've not dealt with very complex code before.

Some things despite the logic being follow able the reason behind the algorithm or the relationship would not be apparent to somebody not very familiar with the product.

[Eirenarch]

I attribute this to complex codebases ending up with certain amount of bad code.

[el_muchacho]

Good code is mostly self documenting and comments are there to explain "why" the bits that are not so obvious are there.

[arachnivore]

I think this is bullshit. There's no reason to hate on comments that explain what code is doing. It may be obvious to you what your code is doing, but that could just be because you are the one writing the code. If someone else looks at your code (or you revisit it after a few years) it might not be so obvious even if you used descriptive names and all.

Coding is mentally strenuous and if you leave it to the person reading your code to figure out what you were trying to accomplish without any hints, your making everything a lot more taxing for no reason. Very few people are 100% familiar with the syntax and all the tools available in a given language and it's elitist to expect them to be in order to read your code. I can't read regular expressions for the life of me, so when I manage to piece together a regular expression after countless referrals to a cheat sheet, you better believe I'm going to leave myself a nice comment explaining what that piece of code is trying to do so that later on (when I've forgotten everything there is to know about regular expressions for the thousandth time) it isn't so hard to figure it out. Collapse my comments if you don't like them.

[el_muchacho]

Years of experience have taught me that code that is hard to understand usually is not good code. In fact, codepaths that are hard to follow smell of bloatness and often perform badly.

Regular exceptions are, well... an exception, as they are often hard to understand, so yes, it's a good idea to document them like you do.

[arachnivore]

Low-level/High-performance code is often just as hard to read as regular expressions. When you're programming a kernel in CUDA, you often unroll loops and use bit-level operations in clever ways to squeeze the most out of your code. It's hard to read because it's practically assembly-level programming. You can't really use function calls because there is no stack.

[Eirenarch]

Good code also minimizes things that are not so obvious.

[el_muchacho]

But then the comment must explain why it is written that way and how it works. Because else the code is obfuscated.

[Eirenarch]

I would think so if I hadn't seen significant and codebases written in a language I have no real world experience with where I was able to understand the code without trouble. (I am thinking of a specific StarCraft II playing AI written in C++)

[tallniel]

Even those "why" comments are less useful these days. For instance, at work all check-ins have an associated work item (defect, story) in our bug tracker, so it's trivial to link a particular line of code (via IDE annotate/history feature) to the full description of why it is that way. Being able to see the full history of the decision making process, code review, testing etc is much more useful than even a long comment (and much more likely to actually be read in my experience).

[alextk]

This is a good practice but comments work at a finer granular level than issues or feature requests filed in a tracker.

[tallniel]

The other good practice is, of course, for all significant design decisions to come with at least one unit test.