42
\$\begingroup\$

I want to ask your opinion on how we should handle submitted question with code that is "fine".

I assume that will be the case for very view of the submitted review requests since everyone likes and dislikes other things and some will have feedback that differs from "seems fine to me".

What I want to avoid is having questions that nobody has suggestions for, and to have them linger around "unanswered" for days. I also don't think it's a good idea to post "seems fine to me" early on since it doesn't add much value once other people start posting.

Maybe this won't be an issue at all?

\$\endgroup\$
2
  • 1
    \$\begingroup\$ This turned out to be a non-problem for my question. The first answer was "it's fine" and while subsequent respondants also agreed it was fine, they did have a number of suggestions that I appreciated. \$\endgroup\$
    – John Dibling
    Commented Jan 27, 2011 at 18:18
  • 3
    \$\begingroup\$ A slightly humourous response: perhaps we should have two tags: nitpick and nitpick-more. They are like compiler warning flags or logger levels. Just as you might set the logger from debug to info, you can set the nitpick level from none to nitpick to nitpick-more. ;-) \$\endgroup\$
    – Tom Morris
    Commented Jan 28, 2011 at 2:12

5 Answers 5

Reset to default
47
\$\begingroup\$

Knowing when to say "That code is done, move on" is just as important a skill as finding fault.

If the code is "fine" someone should say so (as an answer) which can then be voted up (or down) by the community. Why force users to only provide criticism where there may simply be none to be had?

Part of a code review is verifying the implementation and correctness of a code block. Code review is an iterative process. How will users know when their code is "done" if praise is verboten? The suggestion that "It's fine" posts should be discouraged makes little sense.

\$\endgroup\$
4
  • 20
    \$\begingroup\$ I agree that "this is good code" is a legitimate answer. Indeed we should be pointing people to these rare gems. One thing though, if you answer that the code is good, you should say why. \$\endgroup\$
    – LRE
    Commented Jan 22, 2011 at 5:32
  • 5
    \$\begingroup\$ I agree, as long as we can discourage people from posting quick "Looks good" answers to collect a few upvotes before someone takes time to review suggest corrections. \$\endgroup\$
    – BenV
    Commented Jan 22, 2011 at 5:47
  • 2
    \$\begingroup\$ @BenV: Reputation is still earned, in that if you post "Looks good" ... and it's not; you're going to pay for it in the user voting. I don't imagine there will be a lot of "guessing" to play the percentages. Those users simply won't have a good Stack Exchange experience that way. \$\endgroup\$
    – Robert Cartaino
    Commented Jan 22, 2011 at 14:36
  • 7
    \$\begingroup\$ Your answer looks fine to me. \$\endgroup\$
    – Mike Bethany
    Commented Jan 27, 2011 at 21:18
22
\$\begingroup\$

I like LRE's comment so much I am stealing it as an answer:

I agree that "this is good code" is a legitimate answer. Indeed we should be pointing people to these rare gems. One thing though, if you answer that the code is good, you should say why.

"It's fine" is just as bad a review as "it sucks".

WHY?

\$\endgroup\$
1
  • 2
    \$\begingroup\$ I'd have to disagree. "It's fine" is usually said to mean it lacks any glaring inefficiencies/styling oddities/potential bugs. Now if the user asks a question along with the code snippet like, "would this be better written recursively?", then yes, by all means, explain WHY the iterative code "is fine". However, most of the time, I imagine the (possibly implied) question will be, "does this code have any problems" in which case "It's fine" shouldn't need any justification. OTOH, "It sucks" can almost always be augmented by including alternative code snippets. \$\endgroup\$
    – Marty Neal
    Commented Jul 15, 2011 at 19:31
18
\$\begingroup\$

No. It's not ok to just say "Your code looks fine." A good code review gives credit where credit is due. Take this Q & A for example. For nearly two years this sucker zombified. Until someone said, You know what? I'm just going to write something. I'll just list everything that was done well.".

We spend a lot of time and effort telling people what they did wrong. We should also put that same effort into telling people what they did right.

\$\endgroup\$
9
\$\begingroup\$

At the bottom of the question, instead of:

LinkRetagCloseFlag

there could be something along the lines of:

LinkRetagCloseFlagFine

This feature would need, for example, 10 votes from people with, for example, >500 reputation to close it (or disable answers or something).

Just thinking outloud here. :P

\$\endgroup\$
1
  • 5
    \$\begingroup\$ An interesting feature request. \$\endgroup\$
    – Mark Loeser
    Commented Jan 22, 2011 at 4:12
5
\$\begingroup\$

It's probably not going to happen often, but anything is possible. If you really think it is fine, I'd say we come up with a guideline of putting it as a comment to the question, and if it goes a few days, promote that to an answer. Clearly it's going to be manual, but we should discourage everyone from just posting "it's fine" as an immediate answer.

\$\endgroup\$
2
  • \$\begingroup\$ Funny, it's almost what I did here: codereview.stackexchange.com/q/146/90 (I say almost because I didn't actually spend a lot of time reviewing). \$\endgroup\$
    – TryPyPy
    Commented Jan 21, 2011 at 22:11
  • \$\begingroup\$ @TryPyPy that comment lead to me the question :) \$\endgroup\$
    – edorian
    Commented Jan 21, 2011 at 23:31

You must log in to answer this question.

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