6
\$\begingroup\$

I'm surprised to ask this but, "Must answers include a Code Review?"

I'm looking at this answer.

  • The request in the OP says,

    Can you please look into my code and suggest me some performance improvement techniques?

  • The answer says,

    No, it doesn't work like that. ... you should start by profiling your execution ... Either way, first, profile the execution and identify the worst offenders."

\$\endgroup\$
  • 1
    \$\begingroup\$ I had flagged that answer as "not an answer" when I first came across it. \$\endgroup\$ – 200_success Feb 1 '14 at 9:23
7
\$\begingroup\$

In a word: Yes.

Although "Hey, use a profiler!" might be good advice, it's not on-topic or sufficient for this site. This site is for Code Reviews, and an answer on this site is not on-topic if it refuses to review the code.

Instead of being posted as an answer:

  • It can be posted as a helpful comment to the question
  • It can be a vote to close the question (if the question doesn't contain code which can be reviewed)

Another example is:

  • Here I wanted to mention some links to other algorithms, but I didn't want to review the OP code: so I posted that as a comment, not an answer.
  • Here I also posted a link to other algorithms, but I also reviewed the code: so I posted that as an answer.

My policy is similar to the one in this meta-answer regarding answers which contain code, i.e. "Could this answer have been written by reading the text which describes the problem, without reading any of the code in the OP?"

  • Yes, not one word of the answer relates to any word in the code of the OP -- therefore it's not a code review and is off-topic: "It was posted as an answer but does not attempt to answer the question (i.e. a request for a code review1). It should possibly be a comment ..."
  • No, at least one word of the answer relates to the code (not just the description of the problem) -- therefore it's a code review and is on-topic.

1We know the question must have been asking for a code review, otherwise the question wouldn't have been on-topic.

\$\endgroup\$
  • \$\begingroup\$ You convinced me. I made a minor edit and retracted my upvote (don't do this at home!). \$\endgroup\$ – Mathieu Guindon Feb 1 '14 at 2:19
  • 1
    \$\begingroup\$ What is your definition of a Code Review? Is this answer technically a Code Review? If so, in what way? \$\endgroup\$ – Simon Forsberg Feb 1 '14 at 9:04
  • \$\begingroup\$ @SimonAndréForsberg Why wouldn't you consider dead code elimination to be acceptable as code review? \$\endgroup\$ – 200_success Feb 1 '14 at 9:19
  • 1
    \$\begingroup\$ @200_success I do believe the answer I linked to is a good answer, that is why I want to know how to define "Code Review". \$\endgroup\$ – Simon Forsberg Feb 1 '14 at 9:25
  • 1
    \$\begingroup\$ @SimonAndréForsberg I did wonder about that answer at the time: I thought it would have been a great comment! One (strange) reason why it's clearly a Code Review is that the OP doesn't say (in English) that they're implementing a property-bag and property parser; so AJMansfield must have read the OP's code, and reverse-engineered its purpose, before suggesting the alternative: and that deserves credit. If the OP had started with "I'm implementing a 'Property' class using Java" then the answer would have been less impressive IMO, but an even better comment. \$\endgroup\$ – ChrisW Feb 1 '14 at 11:56
  • \$\begingroup\$ Are you suggesting that it's always an adequate review to suggest equivalent pre-made functionality? If someone posts a question about a C++ circular buffer, can we point them to a boost C++ library class? And/or to a non-free vendor's offering? Does the answer depend on whether the OP is doing the code as an exercise, or for production? Is "Is there a built-in API or third-party library which does this?" a valid question, even though that might be seen as 'a shopping question'? \$\endgroup\$ – ChrisW Feb 1 '14 at 12:09
  • \$\begingroup\$ Also my worry about this specific answer was that just "Use a profiler!" might be used as an answer to any question about performance. \$\endgroup\$ – ChrisW Feb 1 '14 at 12:12
  • 1
    \$\begingroup\$ @ChrisW I don't like your "always" questions. I can't say anything about whether or not it's always an answer if you suggest equivalent pre-made functionality. The whole library-issues is an entirely different topic, but if they have re-invented the wheel without knowing it then I think suggesting that "This wheel already exists, here's how you use it yada-yada-yada..." is an OK answer. \$\endgroup\$ – Simon Forsberg Feb 1 '14 at 13:26
  • \$\begingroup\$ @SimonAndréForsberg Yes, "always" can imply a false dichotomy: "Is it always, or never?" I think I ask it in order to test the rule: is it true, and/or are there exceptions to the rule? I think that's a scientific method: "My hypothesis is that answers must contain a review to be on-topic. Is that always true, or is that contradicted by observed counter-examples?" It can be a way of generating useful laws, for example "Always Newtonian Mechanics, except when very fast or very massive or very small or...". \$\endgroup\$ – ChrisW Feb 1 '14 at 13:58
3
\$\begingroup\$

Yes, answers must include a code review, but I'd like to point out that a code review does not need to be long. Any concrete suggestion that would result in an improvement could constitute a review.

\$\endgroup\$

You must log in to answer this question.

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