13
\$\begingroup\$

Sometimes, others have already pointed out the obvious flaws in a review. What is left after that are details: naming conventions, style conventions, things that are not bugs that affect results, things that are not important.

I recently made an answer on a question about a single letter variable. Apparently, people disagreed with that answer because it was a really small detail. Are such details worthy of a separate answer? If not, how should they be communicated?

\$\endgroup\$
  • \$\begingroup\$ "Not only what we say, but also how we say, it is important". It doesn't have to be because you nitpicked on a single letter variable that caused a down-vote, it might be because on how you nitpicked. And what other message you inserted into the post. \$\endgroup\$ – Simon Forsberg Dec 13 '15 at 20:33
  • 1
    \$\begingroup\$ Half of my answers are nitpicking, with one or two real tips, and most of them are well-received. \$\endgroup\$ – Nic Hartley Dec 14 '15 at 17:32
  • \$\begingroup\$ Trust me QPaysTaxes isn't lying. Everything posted is all about nagging. \$\endgroup\$ – 13aal Dec 15 '15 at 13:47
13
\$\begingroup\$

What is left after that are details: naming conventions, style conventions, things that are not bugs that affect results, things that are not important.

I beg to differ. Readability is a major point of concern in both professional and hobbyist code. You code can be good, but how well does the next maintainer understand it?

CPU time may have been expensive one day, but developer time is still expensive now.

\$\endgroup\$
  • 2
    \$\begingroup\$ While I personally agree, I usually try to find multiple such points to make the answer a little more than "Don't use single letters as variable names" \$\endgroup\$ – Vogel612 Dec 13 '15 at 20:28
  • \$\begingroup\$ @Vogel612 Definitely. If you can find more, don't forget to point those out as well. But in this case, all other points were already addressed. \$\endgroup\$ – Mast Dec 13 '15 at 20:28
11
\$\begingroup\$

things that are not bugs that affect results, things that are not important.

If OP's code has so many bugs that affect results that all they're expecting out of a peer review is "where are my bugs?", they're asking the wrong kind of question, on the wrong site. If the question is on-topic there shouldn't be many bugs that affect the results anyway, because to be on-topic your code needs to already work as intended.

That said, of course "nitpicks" are fine answers - there's no way a piece of code is ever fully reviewed without any nitpicks pointed out. Many times I've answered questions by addressing blatant style and naming issues, leaving room for other reviewers to point out the larger problems. Heck, I've even posted pretty long answers addressing one single line of code out of the entire OP's post.

I agree with your answer, except I would have also mentioned s (or is that yet another convention?), and as @Vogel612 points out, it's often a good idea to add some more "meat", or rather, more nitpicks. Let me explain: a nitpick-only answer is fine IMO, when there's more than just one thing mentioned. Otherwise some users might think it looks like it could have been posted as a comment, but answers-as-comments should be discouraged, so while comments as answers are definitely wrong, I see nothing bad about a short answer that points out a problem with OP's naming style. Nothing wrong at all. Upvoted.

\$\endgroup\$
  • 1
    \$\begingroup\$ I think a comment is appropriate for something like "Descriptive variable names will show your intent more clearly than single letter variables" -- if that's really all you have to say. \$\endgroup\$ – Jonah Dec 14 '15 at 16:42
  • 1
    \$\begingroup\$ @Jonah Comments are second class citizens and liable to be deleted. If that advice is actually useful, then it should be an answer so that it can be preserved better. \$\endgroup\$ – SuperBiasedMan Dec 14 '15 at 17:09
  • \$\begingroup\$ @SuperBiasedMan, I think that view is super biased, and i reject it. \$\endgroup\$ – Jonah Dec 14 '15 at 17:44
6
\$\begingroup\$

IMO. As long as it's a "good" answer, I won't ding it for being short. I might be less inclined to upvote, but I don't think it's something to be discouraged.

That being said, there's (usually) nothing to stop you elaborating on a small point to fill out any "minimum length" you think is appropriate.

\$\endgroup\$
6
\$\begingroup\$

What is left after that are details: naming conventions, style conventions, things that are not bugs that affect results, things that are not important.

Details are important too. Maybe you meant not as important as the points in some existing answers.

The issues you pointed out are important. One way or another, somebody should tell the OP, otherwise how will he know. If some users don't appreciate nitpicking answers, don't worry about them. If you are met with hostility, feel free to bring it up in the 2nd monitor chat room, you might easily find supporters.

As @200_success' said in a comment, in reaction to my previous version suggesting to comment instead of answer if the nitpick is very short:

I believe that all suggestions for improvements should be written as answers, not comments, and length should not be a criterion. Comments should be for helping to improve the question. If the OP modifies the code in the question using advice from a comment, I consider it fair game, as long as it hasn't also been mentioned in an answer. Also, comments with bad advice cannot be downvoted, and having an argument thread on the question would be inappropriate. People put quick answers in comments all the time on [english.se], and it's horrible.

@Jonah also made an excellent point:

People will be more open to your nitpicks if you identify them that way. Eg, "I know this may seem like a nitpick, but be careful about the consistency of your indentation. Clean indentation makes your code readable and inviting to other programmers." Versus something like, "First off, your indentation is a mess..."

\$\endgroup\$
  • \$\begingroup\$ I believe that all suggestions for improvements should be written as answers, not comments, and length should not be a criterion. Comments should be for helping to improve the question. If the OP modifies the code in the question using advice from a comment, I consider it fair game, as long as it hasn't also been mentioned in an answer. Also, comments with bad advice cannot be downvoted, and having an argument thread on the question would be inappropriate. People put quick answers in comments all the time on English Language & Usage, and it's horrible. \$\endgroup\$ – 200_success Dec 14 '15 at 4:54
  • 1
    \$\begingroup\$ People will be more open to your nitpicks if you identify them that way. Eg, "I know this may seem like a nitpick, but be careful about the consistency of your indentation. Clean indentation makes your code readable and inviting to other programmers." Versus something like, "First off, your indentation is a mess..." \$\endgroup\$ – Jonah Dec 14 '15 at 16:46
2
\$\begingroup\$

I left you a down-vote on this answer of yours not because I don't like nitpicking or purely stylistic answer (I'm not particularly fond of them, though), I down voted it because I disagreed with the points you were making.

This has nothing to do with the type of answer. I just felt that your answer was really only saying that "k should be key" because it improves readability. I, however, thought that it was still easy on the eyes with that single letter variable.

Also, I disagreed with how you said that OP might be trying to golf or obfuscate their code because, if they were trying to do that, it would be made very obvious in the fact that you probably wouldn't be able to read the code at all.

\$\endgroup\$
2
\$\begingroup\$

The thing about nitpicking is that it often addresses only the form without any relation to the problem that the given piece of code is trying to solve (i.e. without any 'meat', as Mat's Mug put it). There are quite a few answers here on CR where people seem to be working off nit lists, answers that only contain hot air about form.

Perhaps there should be a rule strongly discouraging the picking of nits that can be taken care of automatically by a code formatter (prettifier), and that answers purely about form (even if not automatically fixable, e.g. symbol names) are not overly helpful - unless they also demonstrate how the application of their own advice results in a substantially better expression of the algorithm/function.

Also, some people seem to think that even poor/inadequate conventions are better than no conventions at all (e.g. Delphi, Java). The overriding concern should be clear, concise and precise expression of the algorithm or function under consideration. Clean, lean and mean.

The problem is, of course, that only the prettifier criterion is hard (in the sense of 'well-defined') and the rest is about exercising good judgement... Especially as good code is always about the confluence of form and function.

High-quality code often looks deceivingly obvious and slightly boring, belying the hard labour that went into making it so by trimming the fat, removing noise and distilling it to the essential. I.e. people may think 'Obvious, I coulda done that.' which is precisely where many of them are sadly mistaken.

\$\endgroup\$

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .