TLDR: FRC = OK, at a job usually not good.
I'll chime in on this. Realize that programming for FRC and programming as a job are somewhat different. In FRC, the method being called by the framework is Execute, IsFinished, etc. These methods names tend to not be very descriptive of what is attempting to be done, but you're not doing a lot in the method (5, 10, 15 lines of code). So, adding comments for why and not what your doing is generally always good. I did a code review with the development team on Monday and pointed out some minor things. I asked a the junior/rookie programmers about our vision code. There was a method called FilterContours. When they said what they thought it would do, I asked the developer, does it do that? He said, No. Yes from a low level your using contours, but from a business logic standpoint, using GetBestGoal would have made it better. I usually see nested ifs (
Arrow Code Anti-pattern), Magic Numbers, and unclear variable names. I usually focus on those items in reviews.
In software jobs, comments are usually considered a
Code Smell and indicate a different problem with the code. Typically the comments usually indicate a method has more than one responsibility and the method name doesn't explain what it's trying to do. Another indicator of same problem is that these commented methods usually higher line counts for the method. I'm working on code right now that the following functionality in a single method:
Convert text file to fixed width
Merge fixed width file results into temp file
update several fields
generate statistics
So, as you move to a different block of code, there's a comment. In a code review, I would kick that back. I would tell the developer to break out the commented sections as private methods and then the current method looks like above, instead of 150-200 lines of code. Oh, btw the class is used like the subsystems in FRC, being called by an engine calling methods on an interface. In this case all 150-200 lines of code are in a method called PostProcess.