How important is positive feedback in code reviews?

Question

Is it important to point out the good parts of the code during a code review and the reasons why it is good? Positive feedback might be just as useful for the developer being reviewed and for the others that participate in the review.

We are doing reviews using an online tool, so developers can open reviews for their committed code and others can review their code within a given time period (e.g. 1 week). Others can comment on the code or other reviewer's comments.

Should there be a balance between positive and negative feedback?

Answer 1

Improve Quality and Morale Using Peer Code Reviews
http://www.slideshare.net/SmartBear_Software/improve-quality-and-morale-using-peer-code-reviews

Things Everyone Should Do: Code Review
http://scientopia.org/blogs/goodmath/2011/07/06/things-everyone-should-do-code-review/

Both of these articles state that one of the purposes of code review is to share knowledge about good development techniques, not just find errors.

So I'd say it's very important. Who wants to go to a meeting and only be criticized?

Answer 2

When I do code reviews I tend to just have a running monologue, so as I'm making sense of what I'm reading there will be a lot of "Ok, I see what that does.. Good it connects to this and calls that, alright.. and that piece depends on both of those alright.".

I think in this way it's not "oo la la this is so great!", it could be perfectly trivial boring code, but hearing somebody else actually parse and show comprehension of what you wrote is a form of positive feedback in and of itself, the feedback being "This code makes sense", when I run into parts that I don't understand I ask for explanation and when I do understand it exclaim "Ah, I got it".

I think that simple transfer of comprehension is praise to another engineer because we all want our code to be understood by others, it gives a form of implicit validation.

That said, if you see parts of the code that are good or positive characteristics (even boring trivial code can be good if it's the minimal form of itself) I definitely tend to state those characteristics, again I don't attribute them as "Wow great!" so much as "I see this is a minimal implementation" or "Ok, this complex algorithm has lots of comments", focus on the attributes of the code not so much it's inherent goodness or badness.

Any time you attribute "goodness" or "badness" to code in a code review to avoid making the engineer feel looked down upon or held on a pedestal don't say something is good or bad, but rather talk through the cause and effect of their code.

"Ok this part makes sense, ah there's a magic number here, the meaning of that value might not be well understood by the next engineer to touch this"

"I see you've got a DI container here ok so you'll have loose coupling with that repository"

"Ah there's a static dictionary here, if multiple threads are touching that dictionary we could run into some race conditions"

Notice, I'm not saying anything's good or bad, but whether the engineer should change it or not is going to be understood by the engineer whose code is being reviewed. Obviously you have to end the code review with a yay or nay, but accumulating these statements over the course of it will soften the nay's as explanation has already been made in the form of cause and effect statements when you tell them "I'd like those magic numbers fixed before checking this in".

Answer 3

If I saw something in a code review that I really liked and was above and beyond "good-enough" code, I'd give positive feedback.

In general, I think that if someone writes a piece code that actually makes you say "Wow, this is really nice!" then yes, positive feedback is important - it makes it known to the author that what they did was enjoyed by others, and they should try to do that again. It has to be more than just following guidelines and standard practices though. Giving out praise because someone indented nicely or added boilerplate comments could set the bar rather low.

Answer 4

This isn't so much a programming question as it is a general management and human-interactions question. Positive feedback in code reviews is exactly as important as positive feedback in any sort of review.

Whether or not this is required (and the extent to which it is required) is a function of the disposition and emotional makeup of the person you're talking to. Some people respond to correction much more effectively when it is coupled with praise. Others see praise as insincere when delivered with correction.

The general formula is sometimes called a "Feedback Sandwich": Good stuff first, bad stuff second, good stuff last. The idea is to keep the overall tone positive while at the same time making sure that the negative feedback is received. This can help prevent stress when anticipating a review, and help prevent self-absorbed brooding afterward. Both are very important with respect to productivity and quality. This isn't just touchy-feely emotional hogwash; It's human behavior 101.

Again, you have to know the person you're working with and understand what they respond to. Dealing with people is what management is about, and good managers know how to make people respond.

Answer 5

I think positive feedback is very important and it's mainly from a personal, realpolitik dynamic. We all sit and write code for hours, days, weeks, months, and most of us take pride in what we do. Code reviews are a chance to showcase that.

If you go to a code review and the best outcome you can hope for is "no comment" (i.e. there is no balance of positive feedback), the meeting could easily be titled in outlook "Find Out How Badly People Think You Suck". Consequently, developers will start to be annoyed by or even dread code reviews, and that's clearly a detriment to the team. Developers will "forget" to get their code reviewed or will develop learned helplessness and simply ask their constant critics what to do about every little thing to avoid getting blasted in these meetings.

It's all well and good to say that, theoretically, it's most logical to fix the bad and to ask everyone to leave emotions at the door, but it's precisely attitudes like that one that are responsible for the rep developers get as being interpersonally tone deaf. Theories aside, we're humans and humans like to get a pat on the back from time to time, even a nominal one. That stuff matters.

Answer 6

The only way that I can think of where providing positive feedback about code could backfire on you is if you're not careful to avoid the "backhand compliment." Most people are familiar with this... it's signified by phrases like, "Great job, but..."

If everyone comes to the meeting with the attitude that this is not a personal review of the programmer, but an effort to improve coding practice for the quality of the entire system, then all feedback is "good" feedback. Feedback that highlights ways to improve coding practice becomes just as important as feedback highlighting a useful new method for handling a problem.

At the very least, if one doesn't go to that length, it should be stressed that striving to do a "good feedback, bad feedback, good feedback, bad feedback" cycle within the review process is only going to come across with the same backhand compliment feeling. Don't try to force good feedback, try to reinforce good effort, and shore up holes in knowledge.

Phrases that I've learned the most from, over the years:

• "That's an interesting approach. What happens if we have to accommodate [some other use case]?"
• "Nice try! Did you know we already have a method for doing that? Maybe we should do some benchmarking to see which approach is more efficient."

Answer 7

It's more important if you're doing side-by-side or team reviews. In a written review, no news is good news. The goal is to get code into production. When it's your code, you should feel good about yourself.

Code review should be used as a source of information to help with mentoring and managing of the team. There are plenty of opportunities to give positive feedback without cluttering up the code review database. Examples can be pulled out to share with others.

There is a lot more to reviewing the developer other than their code. Hijacking code review time can be counter-productive to getting an app out the door. Set time that is specific to helping the developer outside of code reviews, but that doesn't mean you should exclude code review feedback.

Answer 8

The workflow that I liked most with code reviews was this:

1. Dev submits patch on mailing list/online tool.
2. Everyone who needs to care looks at the patch, suggests improvements.
3. Dev goes back to #1
4. If there's no improvement needed, people say "Good work, please commit." <- POSITIVE FEEDBACK. All code that is accepted is good. The less people have to tell you to change things, the better you're doing.
5. Dev commits, moves on to the next item.

Usually what would happen is that new devs would get a lot more 'correctional' feedback as they got familiar with the codebase.

The benefits of this approach are:

1. Everyone knows what everyone is doing. There's no knowledge monopolizing or mystery commits.
2. Everyone learns from other's feedback. This is important. If feedback only happens between 2 people in a face-to-face conversation while pairing, then someone on the other side of the room does not benefit the same way as they would if it happened on the mailing list.
3. Other devs can usually spot some bugs before they're in the version control.

Answer 9

It has mattered to me. I don't want fluff comments or positivity for the sake of positivity. If all the code I wrote is crappy, you tell me why and let's correct it and learn. But if I do something right, it's nice to hear it once and awhile. I don't need positive reinforcement for everything I did that was "correct", but even if it's a "let's improve X, Y, and Z, but the rest looks good" it matters.

Answer 10

Not nearly as important as honest feedback. I work for a large financial corporation, and our customers don't care if the programmer is trying hard or is a good guy, or usually write good code! They require software that works.

Answer 11

I can't agree with this at all. What's the difference between Good Development Techniques and so-called Ninja Coders who can write awesome but inexplicable-to-mere-mortals code? Software Development is now (IMO) a Lowest Common Denominator discipline where flair and cunning are shunned in favour of maintainability and ease-of-understanding. It's just too risky.

I can't think of a time I've ever seen code in a review that would make me go 'Oh that's cool'. I can only assume that if I did encounter code like this it'd fall into the Cool-Yet-Unacceptable camp.

You'd also have issues with people who get no positive feedback perhaps trying too hard and making an eventual mess "Trust me, it works!".

Code reviews are there to spread code quality responsibility among the team, i.e. an individual developer cannot be blamed if a serious problem arises later. Use it to find issues, use it to get explanations from the original developer of weird stuff in case you ever end up having to maintain it. Personally, I'm more interested in receiving negative feedback. Customers don't care about the coolness of your code, only that it does what they want.

Leave the backslapping to the pub.

Answer 12

I think it is important to be completely objective. Attempting to boost morale by making positive comments is a waste of time to my mind.

This can mean that code reviews are unduly critical - but then isn't that the point. We should also be critical of ourselves. I find that the assumption that the code that I have written is probably complete rubbish and definitely could be improved drives me to improve my code and my skill level.

If you get no comments then you can consider you have done a good job.

Answer 13

Mantra is simple: If one wants quality Code (which is less in reality) then proper review methods must be practiced. Having said that, positive feedback helps a developer/programmer to think and come up with ideas/solutions/fixes. Don't be too harsh, but be firm on the point. Q&A Managers should be aware of good methodologies and practices so that he/she could guide the team (or a member) in the right direction. This results in quality. Period.

Answer 14

When the code is for a competition or submitted for a job interview (in other words, code that's written and cannot be rewritten), then positive comments are a must. In fact, you should ensure that there is positive feedback (where possible!) as well as negative. That way, the coder knows where his strengths and weaknesses lie, and can compensate.

However, you seem to be talking in a workplace environment, where the code can be rewritten. In which case, you're trying to get bugs the hell out of your system. So, in that situation, only negative bugs are of any worth.

If you feel uncomfortable about that, have a weekly code review meeting, where everyone can discuss both good code and bad code.

EDIT: Though I will say that, if something impresses you enough, there's nothing stopping you from expressing your praise in person. The tracker, however, seems to be only for production code review.