This was presented at Droidcon NYC 2019. See the video here:
Earlier this month, there was a very controversial tweet by the official Java ...
For further actions, you may consider blocking this person and/or reporting abuse
Consider the method:
sendNewsletterEmailOnSundayOkay that's long but you won't need a comment, let's shorten it.
sendNewsEmailSunBetter but can we do more?
emailSunNewsIt's getting harder to shorten and we are starting to loose context. This method could be mailing a customer on Sunday or maybe it's mailing the newspaper news on Sunday. So maybe a comment is needed... Or perhaps the problem is the method wasn't generic enough, bad design.
sendEmail(who, what, when)That way the implementation is clear.
Other things that might help, decorator annotations, type safety and so on.
I prefer
sendEmail(who: Person, what:string, when:Date)kind of thing. It's too verbose, but with clarity. But, we have our preferences in our code. Just, don't put dead code comments in that file.Why did you feel the need to shorten the original method?
Okay seriously, as a developer I feel a compulsive need to express more with less, but not so much less that I can't express my meaning.
In the same way anti comment opinions didn't want to see novels because it gets in the way or perhaps insults thier intelligence, I think it's the former. But most of all I knew this was a bad method, because generic methods should always exist first then helpers can derive from them, with comments explaining what the base method does.
I don't think it's a bad method. If you're only ever sending emails on Sunday, make a method that explains that. Making a generic (who, what, when) for only one use case is more complexity than you really need, IMHO. :) Premature optimization can be a bad thing.
Premature optimization doesn't exist in my book. Over engineering does. But if there are 7 days in a week and a client that might change thier mind. It doesn't hurt to keep it generic. That is where development point of view clashes because I do believe in future proofing but others don't, I get it and I'm cool with it, but I'm going to predict what is reasonable to predict, that's just me.
Somebody once told me "we don't know anything about anything. What we do know time will tell." So that is where my point of view comes from.
Yeah, and sometimes it makes sense because like you said there are 7 days.
But whenever a project manager tells me "we might want to support this" I think really carefully about how likely that is, and whether it's worth over engineering now or just building for the project at hand.
Of course, we are talking about work, work, that means delivery takes precedence and with haste I agree. You get a feeling when something will take too long to explore, that feeling is my limit.
At home I just ignore that feeling and make my scripts do backflips around the internet whistling to the tune of yanky doodle. Now that's over engineering to be proud of.
Peer pressure 😂
Comments are valuable when they enable me to skip over reading a section of code.
Yes, clear, expressive code is important, but sorry to burst egos- I don't want to read every line of even beautiful code. I'm a busy woman.
Write comments to summarize sections & reveal gotchas & important details. It's just more efficient that way.
Agreed. Maybe comments are TLDR; for code? 😎
The linked Medium article still raises a good point, that comments can be seen as an indicator for a special type of code smell. The one that puts a ton of cognitive load on you while trying to make sense of what's going on.
However, in that case the problem is not so much with comments but with the code itself.
I was recently working on an extremely convoluted spaghetti code base that didn't even bother with explaining the obscure if / else if / else if / else sections that spanned over hundreds of lines in a single method. I would have really appreciated comments that explained what was happening there.
I refactored that code and broke out each section that appeared to be doing one unit of work into separate methods (and some of them further down into more methods) with self-describing names. Along the way I found and fixed several serious bugs which were really hard to spot due to the convoluted nature of the original code.
The refactored result doesn't need any comments anymore. Everything is now split into single-purpose methods, and the original main method is now collapsed from several hundred lines to just about 7 method calls, each explaining what's happening. The main reason for that refactor was that I had to write unit-tests for that component. Breaking down complex logic to small single purpose units of work doesn't only make the code more readable but also gives you the extra benefit of easier, more controlled "unit-testability". I wrote for each refactored method a bunch of individual unit test. This would have been very difficult to do while everything was still crammed into the same method.
The Medium article also mentions that kind of refactor and presents it as an example why we should remove comments from code. However, in my case there were no comments in that inherited code base. I was simply dealing with crappy and undocumented code. I would have been extremely grateful for some comments explaining to me what each section did.
There are usually just a handful rare occasions when I decide to add comments to my code. It's when some kind of external system or dependency imposes some odd workarounds that are not obvious to understand, similar to your
HashMapexample.However, I do add (JavaDoc) comments as documentation to each type and method. I also try to make sure to provide them with examples, explain under which circumstances exceptions are thrown or under which circumstances
nullis returned, and so on.I like to generate documentation from these comments, and I find it quite sad and depressing when I only see type and method names but not a single word written in plain English. But that's my OCD speaking. Over the years I learned to tame my OCD a bit, so I also skip documenting simple getters and setters, where there is no point in explaining what's going on 😄
Great article!
Another way to address TODO comments is by incorporating a tool into your code review process that allows the team to prioritize and organize TODO comments. imdone does this by laying them out in a kanban board.
Neat, thanks for sharing that! I have also seen various static analysis tools that can actually fail a continuous integration process if you include TODO comments.
Unfortunately, the real world happens and TODO comments aren't bad. That's why I like suggestions like JIRA and (now) imdone because it helps create that accountability and making sure they aren't lost forever.
I think it comes from blog.codinghorror.com/code-tells-y... which I wholeheartedly agree.
And you're right, going slightly verbose on comments on public interfaces of your API may only be allowed only if it's for documentation generation purpose. Libraries like Typedoc for Typescript (a little bit OOT here) generate documentations extracted from the comments, as other people may have mentioned.
Ahh it was definitely coding horror! Thanks!
I think comments are very valuable for all the reasons you mentioned. As you said, I think the problem is we are just using them wrong.
Comments, when formatted and used with the right tools are also great for generating external documentation.
One thing that I like about the new release of Xcode 11 later in the year is that it as you update your code it can also help by trying to update the documentation for you, I'd like to see more tools start thinking this way.
Yes, Android Studio does this well too. If my comments reference a specific class or variable, refactoring that class or variable will update the comments as well. :) Really helpful in making public libraries that you want to generate documentation for.
Depending on the language features it may not always be this easy, but that's a good idea. :)
My usual approach is to put comments in the spec file of a package to document the API of the package. If what a procedure/function does is quite obvious from the function and parameter names, maybe the comment reduces itself to one or two lines, just for the sake of automatic documentation extractors.
It is really unusual that I put comments in the body part, only if I am using some complex and counter-intuitive algorithms. Often I prefer to comment about the code status in a point by using "executable decorator" such as Assert, Loop_Invariant and contracts.
Sometimes I put a long (sometimes very long) comment at the beginning of the file where I explain what that specific package does and what is the "abstract model" behind it, abstract model that is somehow independent not only on the implementation details, but also on the API details.
An example: you have a package that implements the usual Employee class. In the context of your application an Employee can have a name, a serial number, a base monthly cost and it can be in several states (On_Vacation, At_Work, Ill, Goofing_Off, ...). Note that this model is independent on the API details (how are we going to change the cost? With a method Set_Salary? Or will it be a Apply_Increase?).
In my experience, having a conceptual model of what that package is trying to do helps a lot in understanding its API.