19
\$\begingroup\$

This question is intended to be the counterpart of Checklist for how to write a good Code Review question.

Getting started with Code Review:

New users often seem to have problems to understand, what exactly is expected from a CR-Answer. Many come from Stack Overflow, others maybe from Software Engineering.

These sites favor a different kind of answer than Code Review. I'd suggest providing a guideline or even checklist for new users on how we like our answers best ;)

Some of the most common "mistakes" that new users make:

  1. SO-ish flavoring: alternative code (mostly without explanation)
  2. Software Engineering flavoring: much much explanation, but missing relation to the code at hand.
  3. Trying to go over every Issue in code and then resign over the sheer amount. --> no answer posted
  4. Only "reviewing" the algorithm and not the style (probably also coming from SO), and not posting an answer, because no optimization can be found :(

There already has been a question on what makes a good answer. There was only one answer to that. The problem I have with that answer is, that it only states what a good answer has and not how to write an answer so it becomes good ;)

\$\endgroup\$
  • 1
    \$\begingroup\$ I don't see 2-4 as mistakes. In many cases, explanation without code is good enough. And only reviewing part of the code is okay too, other answers can focus on the remaining parts. \$\endgroup\$ – svick Jun 19 '14 at 11:53
  • \$\begingroup\$ Point 4 is wrong, time ago a question here said the opposite, it's ok to let some parts of the code not reviewed. Sometimes people can talk about the algorithm only and not style because don't know the language (and the same is the opposite) \$\endgroup\$ – Marco Acierno Jun 26 '14 at 0:05
  • \$\begingroup\$ @MarcoAcierno the problem is: Do new users know that? Also, did you read, that I assume there is no answer posted? \$\endgroup\$ – Vogel612 Jun 26 '14 at 7:04
  • 1
    \$\begingroup\$ I don't understand how it could be different because no other answers are posted. I review one part of the code and stop. Then if someone else wants to do the other part ok if not what ia the problem? Noone is forced to review 500 files... \$\endgroup\$ – Marco Acierno Jun 26 '14 at 9:58
  • \$\begingroup\$ @MarcoAcierno You're missing the point. Come to Code Review Chat and I will try to explain better. it's not about "being allowed to stop" it's about "improving the algorithm" or moreover failing to do so and as a result not posting an answer (which is sad :() \$\endgroup\$ – Vogel612 Jun 26 '14 at 10:02
  • 1
    \$\begingroup\$ Related: What makes for a good answer? \$\endgroup\$ – Simon Forsberg Jun 1 '15 at 12:21
24
\$\begingroup\$

There's a few things you might want to follow when reviewing code:

  • Understand the asker's level of expertise:
    Some answers here on codereview are really in-depth. They address issues, that only experts could find, sometimes in ways, that only experts understand. While these answers are awesome, they won't help a complete newbie understand anything.
    Keep in mind, that OP is possibly not as knowledgeable as you, and tailor your answer to their level of expertise.

  • Be a nice reviewer:
    Getting one's code reviewed is ... difficult to start. After all it's your code. You invested (sometimes very much) time and emotions into it. And then there's this ... on the internet saying my code is [swearword here]. How dare he! To prevent OP feeling offended by your answer, you should always be aware about this.
    There's some nice answers explaining a bit more in detail at: How can I be a nice reviewer?

  • Short answers are okay:
    You can find more information in Short answers and code-only answers , but the bottom line is: If your answer can stand on your own, and reviews the code, why not post it? Even multiple answers are okay, as long as each is justified on it's own. Additionally some questions just contain code so extremely bad, that:

    Nobody could catch everything and retain their sanity, simultaneously, with that bad of code

    as Phrancis puts it in Chat, sometimes code just needs a second or third review, simply because of the sheer amount of issues present.

  • Avoid stating absolutes:
    "Your indentation is wrong" doesn't help anyone. It's often better to say something like: "Your indentation is not conform with the [...] standards. These are defined in [link]. You should change your indentation to [...] per level."
    This goes for all stylistic advice, and sometimes even for algorithmic advice.

\$\endgroup\$

You must log in to answer this question.

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