I also find comments do get in the way when they state the obvious, unless they add to the understanding of the code in some way it is best to leave them out IMHO.
[0] https://github.com/behdad/cairo/blob/color-emoji/src/cairo-p...
But then I scrolled down and saw it verbatim just a few lines further down https://github.com/behdad/cairo/blob/color-emoji/src/cairo-p...
Why is CheckMitreHight() not a function? :)
/* img "doc/uml.png" */
I'm sure you can find similar extensions for VSCode or Atom.
Seldom people document the 'Why' which is the most useful some years in as no one knows the 'Why' any longer and might make the wrong decisions in refactoring - also if you don't know the 'Why' you often scratch your head with WTF, though the reasons were sound.
"How" though, very much e.g. reference to the algorithm being implemented or the like if the code is unclear.
Not to mention the misery one feels when they discover that oh, there is in fact no reason at all, or if there ever was, it is now truly lost.
More often than not, you only need a few jumps before reaching the change you're interested in.
I have a 1500LOC files with 1100 revisions right here, "git annotate" processes it in 2.356 seconds on a 2010 MBP, and PyCharm barely takes any longer (maybe 3s).
> How is my IDE supposed to know which of those revisions are the ones that I care about?
It does not care, it gives you a baseline annotation and if that's too recent you drill down into previous annotations.
No thanks. I’d rather have a comment inline in the code than add 3 seconds per file, and still have to go searching through the history.
Or as I've encountered on multiple occasions: your IT team is struggling to figure out why your network/version control/etc are extremely slow... 3 second queries are now taking 60 seconds... we will be rebooting servers a few times... and restoring backups... hopefully things are working better tomorrow!
Or what happens when you migrate to another versioning system? Where I was working migrated from MS SourceSafe years ago and a lot of comment history was lost in the conversion and apparently it wasn't worth the time/money to figure out why only half the comments made it over.
I'm sure there are more scenarios.
The point is: comments in plain-text source files can be invaluable.
Which is exacerbated by everyone's IDE now querying VC for every file!
That's what annotate/blame is for.
> Not to mention the misery one feels when they discover that oh, there is in fact no reason at all, or if there ever was, it is now truly lost.
If there was "no reason at all", it won't be in a comment either.
Except you can't trust comments. Even assuming the comments were relevant at one point, they don't get updated as the code changes, and as new code gets added they can drift away from the original code they were supposed to explain.
Commit messages don't drift, they're immutably bound to the changes the commit performs.
And because they're not bloating up the code it's much easier to be clear and verbose in commit messages than it is in comments, the author can easily explain things which seem obvious to them but may not be so for a reader in a few months or years without lowering the overall readability of the code which is not the case of comments.
People will criticise comments as "obvious" and "unnecessary", I've yet to have a colleague or contributor criticise a detailed commit message for any other reason than the commit message being wrong.
> It should not be detective work to figure out why some code is written in a non-obvious way.
Get proper tools. Browsing annotations and reading commit messages is not "detective work", it's absolutely straightforward and means you're actually using your VCS as something more than a glorified tarballs directory.
That works until the VCS history is lost. For an on-topic example, this usually happens when a formerly closed-source code is released to the public: only the final state of the source code is released, with none of the VCS history. This can also happen when changing to another VCS; often the history is kept only on the former VCS. Some projects have gone through several of these events; I challenge you to find early StarOffice history when trying to understand why some piece of LibreOffice code is written that way.
And there's also the tendency in some places to reference bug tracker issues in commit messages. Said bug trackers tend to have an even shorter life than the VCS. "Fixes issue #1234" is useless when issue 1234 was on the old bug tracker, and both it and the new bug tracker were lost when the company was acquired and migrated to a third bug tracker.
Comments you can at least make an effort to keep up to date. Especially if have a policy that all comments should actually be relevant.
Comments shouldn't be obvious and unnecessary. They should explain things which cannot be inferred from the code itself, eg,
// have to call reset() twice here because the XZW API has a bug which restarts the server if reset is not called twice. See ..."Doing this here because x,y,z, maybe better to do a,b,c later on but would have to do d,e,f" and so on.
I do worry a bit about doing it sometimes because in its own way it admits some sort of shortcomings or such... but still I do it.
When reading other people's code the why is often the question I'm wondering but rarely anyone says.
e.g. This is the kind of thing I find myself wondering very often while browsing our company's codebase:
"Why are we only appending the jurisdiction on this partner and not the others? Is this intentional or an oversight? Maybe the unit tests will tell me more..."
Even if it blends over a little into integration testing, the additional test work (and time to run tests) really pays off in terms of understanding and reusing an old project.
Besides being cleaner, when you add the next vendor, you won’t be searching all over the codebase for if(vendor)’s to add new special cases.
Especially in Java code - look at Android for example. Loads of "setFoo() // sets foo" nonsense.
But in general I think people use the "code shouldn't need comments" thing mostly as an excuse to be lazy and not write comments.
if (status != 0) return -1; // return on error
I prefer that authors spend time to make sure the code is clear about what is happening, and use the comments to tell me why it's happening, and that too only when it's not obvious, or counterintuitive, or there for a special requirement.
Just wanted to point out that comments are not to be confused with documentation as comments, like Javadoc, Godoc strings. These need to be complete and verbose, especially if they code is a library or public.
-1 could mean you're returning a success. I mean that would literally be true in Forth.
Specifically, the message says "return on error", which to me doesn't mean it returns an error, but to you it did.
Also, you are likely to find out that comments like this are often result of people just auto-writing them at the moment e.g. writing it was faster then reflect on it. Which is not necessary good thing, but it is saving time in expense of little bit less effective code.
Regarding time saving, I seriously doubt that code quality is proportional to time spent typing. Instead of typing two things I would prefer the author spent more time thinking and typing just one thing.
// based on stackoverflow.com/questions/201323/how-to-validate-an-email-address-using-a-regular-expression
because I'm not going to pretend to have written this monster myself:
(?:[a-z0-9!#$%&'+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'+/=?^_`{|}~-]+)|"(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21\x23-\x5b\x5d-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])")@(?:(?:[a-z0-9](?:[a-z0-9-][a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-][a-z0-9])?|\[(?:(?:(2(5[0-5]|[0-4][0-9])|1[0-9][0-9]|[1-9]?[0-9]))\.){3}(?:(2(5[0-5]|[0-4][0-9])|1[0-9][0-9]|[1-9]?[0-9])|[a-z0-9-]*[a-z0-9]:(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21-\x5a\x53-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])+)\])
I've only ever done it with stackoverflow because:
1. I think they'll be around for a long time, and 2. they have that policy of explaining answers on the page rather than just referring to external links (yes - irony not lost on me).
Any future dev looking at my 3-line validateEmailAddress() function who wants to learn about it can go to the site and read about it, and maybe find there's a newer status quo way of validating email addresses.
I suppose you could argue that I should copy the explanation of what the regexp does into the comment but really it's just asking for it to become redundant and if someone really wants to get into it they'll probably benefit from reading the entire up-to-date discussion.
Admittedly this is a very specific kind of situation, but this doesn't feel unreasonable or unprofessional to me (bearing in mind that I'm not generally a copy-paster, I prefer to write my own code but in the case of things like email validation I think it's more professional to find an established algorithm than it is to roll your own, unless you have a specific reason for doing so).
This tends to only be for tricky or time-consuming algorithms. RGB to HSL/HSV, is point in a polygon, hash function, etc. Those point-in-polygon functions in particular can be densely optimized to the point that it is not obvious a) how it works, b) what the original algorithm was.
That is why. The person left comment on literally one thing in that code he is able to explain.
if (status != SUCCESS) return ERROR; // return on error
i++; // Increment by 1// gives a thumbs up
user.thumbsUp();
Saves a lot of time vs jumping to the definition to see what it does
I'm basically of the same persuasion. Multi-line comments don't belong in the body of code, single line comments are sometimes necessary but I strive to eliminate them through more self explanatory code (although I will prefer a one line comment over unwieldy long variable names which often outweigh their worth by reducing legibility, especially when talking math).
Beyond that the only good reason I've found for large multi-line comments are when even given the best implementation: when there are subtle details in the concept you are implementing that can appear as being redundant or superfluous in the implementation, it is very important to comment to remind yourself not to break it when next trying to digest it again.
Whatever are your opinions on comments, this file breaks them all with a good reason behind each. And it's still implementing a standard which should be fairly well defined, but... isn't.
So yes, multiple comments explaining code, explaining behaviour, reproducing relevant parts of the standard, and many others. The code both requires them to know wtf is going on, and at the same time is pretty close to simple as far as multithreded SIP can go.