I have working code that I want reviewed, but it is not clean. It made the correct output but there are comments and bad names. Is it not ready for review, and must the code be clean to get reviewed here?
\$\begingroup\$
\$\endgroup\$
1
asked May 24, 2016 at 0:30
-
6\$\begingroup\$ See how to get the best value out of Code Review - your code doesn't have to "be clean", but you'll usually get more useful and valuable answers if you weed out the low-hanging fruit first. \$\endgroup\$– Mathieu Guindon ModCommented May 24, 2016 at 2:27
Add a comment
|
4 Answers
\$\begingroup\$
\$\endgroup\$
No it doesn't have to be clean.
It can get reviewed, but people will point those problems out. If you know they are bad you should probably clean them, that way we won't badger on about them, and more about bits you don't know how to improve.
So it's best if you do clean it, but not a requirement.
\$\begingroup\$
\$\endgroup\$
3
Make the code as clean as you can before submitting it for review.
The more work you put into your code, the more you're likely to get out of review.
That's because reviewers (especially the volunteers here) have a limited amount of time and energy to put into a review. If they have to spend their time pointing out basic issues such as lack of documentation, misleadingly named variables, lack of functional decomposition, and so on (issues that you are probably already aware of), they are going to run out of time and enthusiasm before they consider more advanced issues.
If there's a problem with your code that you are aware of, and you submit the code for review without fixing it, then you're wasting everyone's time. Fix the problems first.
-
1\$\begingroup\$ "reviewers (especially the volunteers here)" - we have non-volunteer reviewers? \$\endgroup\$– PimgdCommented Jun 4, 2016 at 23:23
-
2\$\begingroup\$ @Pimgd I guess, there are some people obsessed with reviewing and calling them volunteers would be a bit imprecise. :D But seriously, this answer is valid for reviewing in general (also as part of a job). \$\endgroup\$ Commented Jun 5, 2016 at 3:54
-
1\$\begingroup\$ @Pimgd: I was trying to contrast the volunteer reviewers here with professional reviewers elsewhere. The same issue applies in business — it wastes everyone's time and money to submit code for review when it's not ready. \$\endgroup\$ Commented Jun 5, 2016 at 8:53
\$\begingroup\$
\$\endgroup\$
I generally agree with the first answer, but depending on the comments, people may point out something like "comments that do this indicate that the code is not ready for review." For example, if you have commented out code or comments that say things like "TODO: implement this feature".
That said, unclean code is not a close reason. Incomplete code might be.
\$\begingroup\$
\$\endgroup\$
This code can be posted here, but A) if you know there is a problem, you should fix it first, and B) comments are good as long as they are useful and tell the programmer something the code doesn't tell them. Examples of good comments are those explaining what the code is doing if it is doing something particularly unusual or unreadable that cannot be changed, or a detailed outline of the spec for a certain method explaining what it does and all the edge cases it must account for.
For example, if the code is doing something particularly nasty that is required for speed or another good reason, then there should be a comment explaining why it is doing it that way so nobody else changes it to a slower, more readable version.
If you have a method that must handle a situation with many slight variations, a good comment could be much longer than the method and will explain what exactly has to be done so it won't be changed to a "better" version that breaks one of the less common usages.