7 ms·
"There are only a few comments in the version 1.0 source code, most of which are associated with assembly language snippets. That said, the lack of comments is
by boznz 8y ago
"There are only a few comments in the version 1.0 source code, most of which are associated with assembly language snippets. That said, the lack of comments is simply not an issue. This code is so literate, so easy to read, that comments might even have gotten in the way."
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.
- tzahola 8y agoif (status != 0) return -1; // return on error I always get angry when I find comments like this in our codebase.
- ak39 8y agoWhy would a comment like that make you angry?
- spacenick88 8y agoBecause it adds no value
- sudhirj 8y agoBecause it re-states what the code is saying, which makes it two things you wind up reading to absorb one fact. It also create two update points if that code needs be changed. 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.
- deleted 8y ago[deleted]
- zeth___ 8y agoIt doesn't. -1 could mean you're returning a success. I mean that would literally be true in Forth.
- yoz-y 8y agoIf the function returns magic numbers then that should be stated in the documentation comment.
- icebraining 8y agoYou're showing another problem with these comments: they're often ambiguous. Specifically, the message says "return on error", which to me doesn't mean it returns an error, but to you it did.
- watwut 8y agoIt just seem quite petty thing to get angry about. Reading those two things takes less then additional half second and on update you can delete the comment if you don't like it. I have seen project behind schedule and it was never because of tiny things like this. 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.
- sudhirj 8y agoThe comment is more a symptom than a cause of problems. If useless comments are added, they’re often an excuse for lazy code, in that it allows a way to say I can’t be bothered to write this better, I’ll just add a comment instead. 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.
- mrweasel 8y agoI had a coworker who would write comments like that, it wouldn’t exactly anger me, but it was a bit strange. Why would you go through the trouble of writting a comment like that, but not leave any hints as to what the devil the 50 lines of spagetti code above does? Because that would always be the case: some obvious comment, and nothing for the horribel or complicated stuff.
- aaronbrethorst 8y ago(they didn't understand what their 50 lines of spaghetti did...otherwise they would've written a pithy comment explaining it.)
- SmellyGeekBoy 8y agoIndeed. "// This was copy and pasted from Stack Overflow. I have no idea how it works." doesn't come across as very professional.
- jclulow 8y agoI believe that comment is required by the licence terms of code cribbed from Stack Overflow, though.
- firmgently 8y agoI've on rare occasion written a comment along the lines of: // 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).
- tzahola 8y agoBecause comments like that accompany the most trivial bits of the codebase, while the thousand line monkey-patched crap has zero comments.
- wk_end 8y agoThis isn't as bad as it could get because it's using magic numbers. If it read: if (status != SUCCESS) return ERROR; // return on error then you'd have a problem.
- sudhirj 8y agoThis is ideal, yes. Would also settle for a previous comment explaining what each int represented, if the language in question doesn't have enums.
- xg15 8y agoTo play devil's advocate (and be really nitpicky), it doesn't say "return an error" but "return on error" which does contain some useful information: That this method doesn't have any kind of sophisticated error handling but simply aborts on error.
- efdee 8y agoI don't think that's very useful - you see that it doesn't have any kind of sophisticated error handling simply by looking at the method's source code (which you need to do in order to see the comment).
- NullPrefix 8y ago0 and -1 as status numbers aren't that unusual to call magic.
- blattimwind 8y agoError handling is one of the areas where ime more comments would frequently be in order, though not in your trivial snippet.
- jenscow 8y agoi++; // Increment by 1
- amelius 8y agoPerhaps someone could write a program that writes trivial comments like that. It would allow me to tell my boss that my code is commented :)
- JustSomeNobody 8y agoI sigh often when I read (bad) code. I don't get angry. That, to me, seems to be a bit much. I'm not trying to be judgemental, I just truly don't understand why anger.
- IshKebab 8y agoSure but the only comments I really find like that are function-level comments where there's a rule that every function must have a comment, so people just duplicate the function name as a comment. 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.
- collyw 8y agoIn my previous job my team leader had commented the "open" file statement (pretty bloody obvious), but didn't bother to comment the seek(8) command after to explain what the hell was happening at byte 8. It was like an example of how not to comment code.
- aidos 8y agoThis [0] is one of my all time favourite comments. Chris Wilson explains how the stroke miter limit works in cairio graphics through the medium of ascii art (and tidy maths). [0] https://github.com/behdad/cairo/blob/color-emoji/src/cairo-path-stroke-polygon.c#L419 https://github.com/behdad/cairo/blob/color-emoji/src/cairo-p...
- 2Pacalypse- 8y agoHa! That's pretty neat. I also resort to ASCII art comments, when appropriate: https://github.com/ShieldBattery/node-interval-tree/blob/master/index.ts#L82 https://github.com/ShieldBattery/node-interval-tree/blob/mas...
- willvarfar 8y agoWhat a lovely comment, explaining not just the why but also, in this case, the 'how' because its not obvious how to derive the code from the requirement so actually explaining how is worth it. 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-path-stroke-polygon.c#L603 https://github.com/behdad/cairo/blob/color-emoji/src/cairo-p... Why is CheckMitreHight() not a function? :)
- aidos 8y agoAnd a bunch of other places in the codebase from memory (cairo-path-stroke.c, cairo-path-stroke-polygon.c and cairo-path-stroke-tristrip.c). And there's a bug in there too because they're working in the wrong space so the checks are applied after outer transformations [0].... but it's still a great comment! :-) [0] https://bugs.freedesktop.org/show_bug.cgi?id=100703 https://bugs.freedesktop.org/show_bug.cgi?id=100703
- criddell 8y agoI wish my IDE could interpret a special comment like /* img "doc/uml.png" */ or something similar that it would use to display an image inline with my source code. There are so many times I've created a FSM diagram with graphviz + dot that I'd love to include inline.
- _Codemonkeyism 8y agoMany people write the 'What' in comments, which is redundant, as we can see the code. Some document the 'How' but this can be deduced from the code except when using very clever algorithms. 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.
- masklinn 8y agoI prefer having the "why" in commit messages than in comments, unless it's something really screwy and needs a big warning right there. "How" though, very much e.g. reference to the algorithm being implemented or the like if the code is unclear.
- xyzzy_plugh 8y agoSpending copious time spelunking through commit logs to find the reason something exists would be better replaced by a succinct comment pointing the reader in the right direction. 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.
- edejong 8y agoMy IDE allows me to easily annotate each line with author and commit message. Invaluable information.
- catmanjan 8y agoWhat if someone changes the line? You lose the useful commit message
- masklinn 8y agoVCS UI generally have a way to re-annotate the previous revision (even "git gui" does), if the code was changed since the original commit but the change is not what I'm concerned with. More often than not, you only need a few jumps before reaching the change you're interested in.
- ilitirit 8y agoSometimes the code is perfectly easy to understand, but the reasons for the code may still be a complete mystery. 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..."
- acutesoftware 8y agoI think unit testing is underrated in this area - no documentation can be better than some additional unit tests that show exactly how to use the code in the real world. 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.
- ec109685 8y agoI think it would be better in this case to add a flag to the vendor that indicates it needs the jurisdiction added and the flag should be named for the reason. 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.
- tomxor 8y ago> 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. 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.
- viraptor 8y agoI'd like to share with you my personal hell for a few years: https://raw.githubusercontent.com/asterisk/asterisk/master/channels/chan_sip.c https://raw.githubusercontent.com/asterisk/asterisk/master/c... 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.
- odammit 8y agoMy favorite comments are the comments right above a function call that says what the function call does. Always a solid thumbs up moment. // gives a thumbs up user.thumbsUp();
- megaman22 8y agoIf for no other reason, I have gotten in the habit of doc'ing my fields, properties and functions with javadoc/xmldoc just because my IDE has such good support for popup documentation when I hover over an identifier. Saves a lot of time vs jumping to the definition to see what it does
- welly 8y agoOften comments in code are used to generate documentation, so without those comments your function won't be documented.
- odammit 8y agoI’m talking inline in a function call. Not documentation above the function.
- bytematic 8y agoClean Code is a great book if anyone is interested in commenting guides.