Code Reviews tend to be a bit of a negative thing. We spend a lot of time pointing out problems in other people's code (and rightfully so). However, a good code review also points out what was done well. Should we put more effort into telling people what they've done right too? What are the benefits of it?
We are all here to write good code. We should be told when we have succeeded in that!
One day I would really like to write something like:
You have made a nice frobnicator here, I like how you used the fizzler to buzz it. Additionally, your naming is awesome and your method interfaces are cleanly written. Now go out there and write some more awesome code for me to review!
There is just something special about hearing things such as:
- This is a good start (the first time you use a language you've never used before)
- looks like an elegant solution
- your code is 250 times faster
- What you made is impressive
- There is not much to criticize in your code.... and there is a lot of code to go though
Even if all the rest of what you are saying is negative (whether it's very negative or just nitpicks) then you can always say that they got it to work, which is an important achievement itself
Sometimes when there are truly no positive things to say then you can say that you would like them to come back, because you would like to write a more positive review next time.
One of my all time favorite quotes is:
Remember compliments you receive, forget the insults. If you succeed in doing this, tell me how
- Baz Luhrmann "Everybody's Free (To Wear Sunscreen)"
If we give more compliments, it will be easier to do that.
When someone asks about a very specific part of their code, I think one of three options are appropriate:
- Don't address it at all because you're not sure about that specific part.
- Address it and tell them how it could be better (because it needs work).
- Address it and relieve their concerns by telling them they've done it in the most appropriate way possible.
However, we should be careful about the third option.
By using option 1 and not addressing it, we effectively tell the asker "I can't see a better way to do that," and at least assure him that someone else agrees that's an all right approach. However, it doesn't confidently lock him into that view point.
If I know two approaches, approach A and approach B, and the asker has used approach A, but I know approach B is objectively better (easier to read, runs faster, uses less memory, best practice, more common way for the language, etc), then I can offer approach B as an answer.
In fact, I can offer approach B as a definite improvement over the current approach without claiming approach B to be the best approach available (there may be an unknown-to-me approach C).
However, if the ask has used approach B, and again, I know approach A and approach B, and approach B is objectively better than approach A, what should I do?
I could comment and say "Well, that's better than approach A"... but how constructive is that?
Variables with actual names are better than variables with single-letter names. Is it useful for me to point this out to every questioner who managed to use more than one character for his variable names?
And here's the problem. What if there's an even better way of naming those variables, but I, with my limited experience with the language, or with what he's doing, cannot see it. To me, he's used the best possible variable names. To another, his variable names are still just as bad because they see these other variable name options out there.
I'm getting rambly...
If you take nothing else from this answer, the point is, given Approach A and Approach B, it's easy to know for sure that Approach B is better than Approach A, but it's very difficult to know whether there's an unknown Approach C out there that's better than either, and that makes telling a user that what they've done is good very complicated.
And if you're ever reading one of my answers (or answer sets), you should take omission of commentary on particular aspects of the question-code to mean "I know no better way."