How do you encourage students to take criticism of their code constructively?

Question

How do you encourage students to benefit from code reviews you give them, rather than being discouraged or responding defensively?

Answer 1

Here a few techniques I use in my class when giving feedback on student code:

• Use a rubric. CS50 has a clear grading approach: code is assessed along the four axes of scope, correctness, design, and style. Within style for example, I might focus particularly on readability vis-à-vis indentation. If a student is marked down on this item, she knows exactly what she missed and why she may have lost points. The clearer the criteria of evaluation, the better the student response to being assessed on said criteria. I also think it's valuable to share rubrics in advance. Only seeing it after the fact can make students feel like how they were being graded was being kept a secret, which can lead to resentment of feedback.

• Avoid "you" statements. This is something I caught myself doing and tried to correct as the year went on. Let's say I'm reviewing a student's code, and I'm commenting on the naming of variables. I might be tempted to say, "You have unclear variable names here." That frames it as an assessment of the student, not of the code, which does not set the right dynamic. I try to make sure I say something like this now: "Your program has unclear variable names here" or "to improve your code, make your program's variable names more precise." The goal is give feedback on the code, not the coder, so language should reflect that focus. (Side note: with how personally code reviews can be taken, I think how we frame the language of assessment is essential to building proper classroom culture when it comes to grades and feedback.)

• Reward revision. This one is possibly more applicable at the high school level, but I think it does matter in terms of building a culture of receptiveness to code review feedback. Allowing students to revise their work gave me the ability to see students improve their work based on my commentary. In my mind this is what education is all about: inspiring a continual learning process. The pragmatics will vary from context to context, but requiring some sort of revision or reflection or allowing students to "recover" some/all lost points communicates to them that at the end of the day, you want them to be better programmers. Grades take a backseat to learning. Again, this might make less sense for university students, but in my class this approach goes a long way in terms of students constantly working on improving their craft.

Answer 2

My year 4 teacher at primary school taught me how to shade rounded objects - by doing so on a picture I had been working on for quite some time.
I recognise now that it did improve the picture, but I was furious at the time; because I had lost control of my work, and I didn't want or feel the need for the help at that time.

There are two points that I think this exemplifies:

• Don't take control of the work away from the student. It can prevent them from wanting to improve it, as it isn't 'theirs' any more
• Show them what can be improved, and how to do it, to encourage them to recognise faults and improve their own work.

One thing you could do, if you see common problems, is to generate your own examples of poor quality code, and get them to improve it themselves. In doing so, they will learn to recognise the problems with their own work.

If feasible, find something that requires criticism to be taken on-board to show them the positives that come from improving their code - e.g. if your students are having problems with correct indentation: "This week class, we are going to introduce ourselves to python".

Answer 3

I have always enjoyed the fact that as a programming teacher I have almost no need to invest time into building up authority (at least not when teaching young adults). Unlike many other fields we always have a computer at hand that will tell us about the mistakes we make, and as soon as we are talking about more complex topics like code design there is always an explanation why something is considered good or bad style.

The result is that I don't see any need to express negativity in my code reviews. I know that good programmers will think of sub-optimal code as horrible mistakes, that's what drives us to improve our code constantly and not be fine with anything that works. But for students getting something to run and do what they want it to is often a huge achievement, not just subjectively but also objectively. If the code doesn't do what they want they will be open for suggestions anyway, so this question will most likely be about situations where the student may argue that "it's working, so why bother listen to your advice?".

The solution I found for myself is to avoid arbitrary rules. Don't tell students what they "have to do" unless they actually have to do it in order to get something to run. Otherwise they will immediately see that what you are telling them is factually wrong, since they didn't do it your way and it still runs, so apparently they didn't "have to do" it.

Instead try to illustrate the upsides and downsides of their solution and your alternative solution. In most cases it's easy to come up with a simple test that allows you to demonstrate the benefits of your suggestions. For example if it's about code design have the student rewrite a short paragraph according to your suggestions and then ask someone who doesn't know the code and wasn't involved in the conversation to summarize what the code does. Usually the improvement will be quite obvious. If your advice was good and you communicated it the right way the student will be proud that they were able to improve their code, since the whole demonstration revolved around the code the student rewrote and the reaction they got. You and your authority (i.e. the things that might trigger pride and make the student eager to prove you wrong) are out of the equation.

If it's not about code design but about things like architecture illustrate it with a reasonable change request that would be more difficult to implement with the "bad" architecture you want to argue against.

Of course tests like these will fail sometimes and the "improved" code will not work better, but if that's the case and you don't see a flaw in the way your suggestions were implemented, then this may actually teach you that your advice isn't as powerful as you thought it was, and reflecting that to the students in a mature way will even improve the respect the students have, because you are demonstrating that it's not you against them, it's all of you against the hard facts.

Answer 4

Feedback can be intrinsically threatening, and people are open to feedback only under the right conditions. Here is an excellent article that discusses the psychology of feedback. In their summary, feedback can be received when:

• The feedback provider is credible in the eyes of the feedback recipient.
• The feedback provider is trusted by the feedback recipient.
• The feedback is conveyed with good intentions.
• The timing and the circumstances of giving the feedback are appropriate.
• The feedback is given in an interactive manner.
• The feedback message is clear.
• The feedback is helpful to the recipient.

As for the classroom context, I believe that the two most important things have already been said by Peter and Buffy: keep the focus on the code, not on the student, and make sure that you have a warm relationship with your students.

What little piece I would add would be that, given the above, if the things you are asking your students to do have self-apparent value, then the resistance never appears in the first place. If they don't have self-apparent value, then asking the question "how can I make sure my students are receptive to what I have to say" is the wrong question, something more fundamental is missing in the presentation.

My biggest victory in this regard was when I figured out how to teach about coding style. I could not for the life of me figure out how to present it as something with intrinsic value. The code that we worked on was all short enough that it hardly mattered, the kids were perfectly able to understand their own code, and my protestations aside, the kids couldn't see any good reason to put thought and effort into indentation, variable names, function names, clean organization, etc.

I outlined how I eventually solved that problem here. By the time I finally had it sorted out, the kids started to see intrinsic value to clarity in their code, so the suggestions I gave to help them achieve that end were no longer met with resistance.

Answer 5

I have all of my students read Keeping Perspective, a section of So you want a code review..., which I wrote when working on an open source project. It's short enough that I'll excerpt it here:

Keeping Perspective

I began working at Google when on sabbatical after having been granted tenure early at Mills College, where I had been working since becoming MIT$^3$ (SB, SM, and PhD). As a professor, I had been the sole judge of others’ code. I expected to sail through my first code review and hear that they had never seen such excellent code before.

That’s not what happened. To give you an idea of what did, here is the art that was created from my suffering, since reshared by many other Googlers:

The Five Stages of Dealing with a Code Review

1. Disbelief: "How could someone find so much to complain about in my code?!"
2. Anger: "How dare someone suggest that a hash-map of hash-maps isn't an efficient way of storing data?"
3. Bargaining: "Maybe if I add the comments the reviewer requested in one file, I won't have to fix the tricky Unicode issues in another."
4. Depression: "There was so much wrong with my code. How can I show my face to the person who reviewed my code?"
5. Acceptance: "Wow, my code is a lot better due to the feedback. This is a useful process, although I bet it's occasionally painful for everyone."

It’s much better not to take suggestions personally.

I also try to model an openness to feedback by soliciting suggestions on how to improve my teaching and putting them into practice immediately. (To see how well I succeed at this goal, I add this question to the end-of-semester evaluations: "The instructor responds well to and incorporates student feedback.")

I begin code reviews by congratulating students on what they've accomplished and telling them I'll show them how to make their code even better (as suggested by kayleeFrye_onDeck).

Answer 6

I would stress that code reviews occur frequently in industry, both formal and informal. Even senior software developers have code reviews. They are not just for improving code quality. Code reviews also communicate changes and teach other developers. One important lesson I learned early on in my career is that there's always someone who knows more than you. You are hurting yourself if you don't learn from them. Of course it shouldn't be all criticism. I often highlight innovative, elegant, and efficient coding examples. The Magic Ratio apples in classrooms too.

Answer 7

There are both personal and technical aspects to this question. First, on a personal level:

Do you have an informal, friendly relationship with your students, or a formal one. If your relationship is strictly formal and you are seen as an "authority figure" you need to be much more careful here. I could do things with students that, for example, my very proper European dissertation advisor could never get away with. Even if your students think of you as a bit goofy ("tells eye-rolling jokes", "very punny") you can do dramatic things. Part of the mythology around my teaching persona is that I once held my nose and dropped a student paper into the trash. I don't think it happened, actually, but it was the kind of thing that "might" have happened. If the personal relationship is strong you can move upward from that and your message will be remembered.

However, being constantly dramatic is probably counterproductive, as it loses its force if used too often.

If you are more formal, or a new teacher without a myth (yet), use more structured means, such as, for example, what @Peter♦ suggests here.

On the more technical side you can prepare ahead some reusable materials that you can use to make various points. If a student is using terrible variable names (a former student of mine used NSFW references exclusively) give them a program in which you have obfuscated names (v1, v2, ...) and ask for an analysis. Two pages of code will be enough. If the problem is poor structure, give them one with (much) worse structure and ask for an analysis. In other words, show them, in a dramatic way, the effect of the kind of errors they are making. If you are teaching OO Java, for example, a program with the "one true method" solving all, can be pretty dramatic. Nest those selection statements to the max.

Such programs can have use year after year, so you can amortize the development cost. You can even use old student programs that have been disguised, perhaps.

If the problem is a failure to meet the requirements, giving them a few tests to pass can help, but this may be harder to amortize.

If you have industry experience an/or connections, perhaps you can show them samples of poor coding that had an effect on the organization. If you still have colleagues there (or former students) perhaps they can help with samples.

Echoing @Peter♦, make it about the work, not about the student.

---

In a slightly different direction, I think that you can't "tell" a student to "not be discouraged." It isn't something you can teach with words, especially in the moment of need. You can talk about it in general before the need arises, of course. However, when the student needs specific guidance, it isn't likely to be useful to say "this is terrible, but don't be discouraged". You have to get them to Embrace Correction. And if you Embrace Correction yourself, accepting feedback from students and acting on it, it can help. At the moment of need, advise so that emotion and personal feelings don't come in to the picture. You want the student to carry away "my program can be improved", not "I need improvement." < opinion >

Answer 8

Being a relatively young tutor (and soon to be informal teacher), I have quite a few examples of bad code that I wrote not too long ago.

If you have no such code then you can simply write some (purposefully buggy and bad), then show it to your students.

The point of this is to explain that had you not taken criticism of that code constructively, your own learning would be impaired. Because one never actually stops learning, I'm sure you can find recent examples of this and use them. Why, even this site might be an example. If you received criticism on an answer or question, then to actually "learn from your mistakes" you'd have to take criticism constructively.

Also, it's important to make them see that without criticism, they have no feedback, and with no feedback whatsoever, it is literally impossible to learn (I'll also add that if they know a bit of machine learning or neural networks then you can use that example: without feedback, neural networks learn so slowly).

It's also important that you give your criticism in a constructive way (I'm sure you do but one should keep it in mind) and then explain to the students that criticism should be taken constructively, because it means there's room for improvement, and the criticism is the key to opening the door to that room.

I saw many students (that studied with me or that I tutored) who were encouraged to take criticism constructively because they were shown by the teacher (who was sometimes myself) just how important it is.

Answer 9

Tl/Dr: Interact with each student individually. Also, I wrote this answer mostly because I wanted to have fun with the list of students at the end. It probably shows, but hopefully it's useful!

So I haven't seen your interaction style with your students. I don't know where the challenges exist. However, one thing I have found to be generally true is that the phrase "take criticism constructively" is a tricky one. It's so easy to put all the onus on the receiver of the criticism, insisting that they take it "constructively."

I think some of what makes it hard is that the phrase doesn't include the provider of the criticism. It, instead, focuses on the "criticism," like it's already out in the open. Now the ability to take criticism like this and turn it into something constructive is an important life skill, but the thing about life skills is that they take a lifetime to develop. You might have a few hours with the students for a year, if that. You can show them the path towards how to take criticism constructively, but you can't expect them to walk all that far down it in a year.

The key, like anything else in teaching or the rest of life, is individual attention to the students. You have to look at what's going on in their minds. And, of course, I strive to say this with the utmost humility: for me, this is a life skill that's going to take a lifetime... and sometimes it feels like it'll take two or three. But that's okay. I still feel like I can say the phrase and have the phrase be true, even when I strugle with doing it.

What I have found very effective is to take the word "constructively" literally and see how far I can run with it. Consider the "stationary" points in the mind of a young student -- the things which are moving slow enough to be worth directing. Some of them are already constructed. They're stationary things like facts and beliefs which are already there. You can't constructively criticize those, because they're already constructed. They're done. All you can do to change them is be destructive. All you can do is tear them down and then try to show them what they should build up in its place. Doing so is certainly part of teaching, but it's not "constructive criticism."

The other stationary point is more interesting, and far harder to see. These are the energetic holes in their lives where something could be constructed. These happen during growing up, and they also happen when an individual decides one of their beliefs shouldn't be so solid and tears it up. They then hold onto that potential looking for something to build in its place. This is where constructive criticism is tremendously powerful. If you can resolve something in their mind just by suggesting somewhere to put their load, the students will rapidly accept your offer.

Note that this version of constructive criticism requires something in their mind already -- a desire to construct. You have to look for where they want to construct things, and make it happen. Consider the following students:

• Alice has already decided she's going to be a programmer when she grows up. Accordingly, she's already got a thirst to learn from you simply because you're the CS teacher. A student such as this may give you great opportunities for constructive criticism. You may be able to provide her "breadcrumbs" guiding her and refining her skills, each time just barely within her reach. You might suggest she look at design patterns, when the rest of the class is struggling over private vs public. Also, because she's already interested, she's going to be inspecting anything along her career path carefully. If you help her put something down, and it lands in the wrong place, she will unearth it and try again. Give her suggestions about how to shoot for the stars, and she'll construct a bridge to get there.
• Bob is in the process of going through lots of life changes. Everything is in flux for him. Maybe his voice cracks, maybe his code doesn't compile on the latest release of RHEL. Bob might be interested in finding some stability from the course. Perhaps you can show him how programming can help him think in ways which anchors pieces of his life. Perhaps you can show him how programming is a stable life that he might enjoy. Give him suggestions about how to build rock-solid code which won't break.
• Charlie is the outsider in this communication. He's just the football jock who got stuck in a CS course. Maybe he doesn't really need programming at all. However, he does need leadership skills. See if there's a way to structure a group project such that having a strong leader is useful, even if they're not all that strong at writing software. Perhaps there's some customer interactions that need to take place? I mean, we all know the customer is a mythical beast whose feet never touch the ground, right? As it turns out, in customer relationships, there's rarely a "right" answer nailed down in stone. Everything has some potential to change, which makes constructive criticism easier.
• Eve. There's always an Eve. Slightly malicious, always trying to find ways to cheat. Eves are tough. That's why she's always trying to eavesdrop on our public key encryption. But everyone is human. I'd say find your own way for dealing with each Eve, but if you can remember that they're human, it's certainly possible to make it work.

Answer 10

I think this question boils down into two generic parts:

1. How to criticise in a way that doesn't trigger defensive behaviour, and;
2. How to teach them how to handle criticism

I think the first port of call would be to bring up the subject of handling criticism as a dedicated class session, including how to extract useful feedback from poor criticism, and how to make criticism (which they would be expected to do in the event of a code review, both reviewer and reviewee). They're adults, bring them on the level of both giving and receiving, and they should understand both sides.

On the other side of the fence, how to criticise I find boils down into two categories:

1. Subjective (EG this could look nicer)
2. Objective (EG if you don't implement this, it will crash, and bad things will happen)

Subjective

On subjective criticisms, I find making 'speculative' recommendations passively works best, especially with reasoning listed. So say Joey forces all his code onto a single line, you'd say:

Joey, I've noticed that in this code, everything is on a single line! I think maybe - just a suggestion - having it on multiple lines might make it easier for other people to read.

Joey has a couple of choices here. He can consider you've got a fair point, and might go for it, or, better yet, Joey will present the list of arguments as to why he has it on a single line (maybe 'no-one else will see my code' or 'I'm doing a contest on fewest bytes used on code gold'). At which point you can either explain why not building it for other people in mind is bad, or concede there is a reasoning behind it.

This approach I find is excellent in team situations because the suggestions aren't forced, and it allows people to argue for/against a particular criticism. A dialogue occurs, instead of an argument.

Objective

This is where real problems begin. It's not because Joey has made an error, but if he continues with the error, stuff will catch on fire, the world is going to end and goto loops will become fashionable again. As a result, you're required to put your foot down.

Before approaching, I find it works extremely well if the opening dialogue starts 'this is nothing personal, but...'. Psychologically, people think it's reverse psychology and thus terrible, but when you point to the code, this helps reinforce it's actually not.

The criticism should be kept short, matter-of-fact 'if this happens it will do this' (if this has no try-catch and it throws an exception, the stack is going to crash and the system will go boom). Explain what-why and how to fix it.

You can help reduce the sense of it being a personal failure (the main driver of being defensive) by either indicating it's a common problem or that it's a hard problem to spot. Joey might still get defensive, however: trick is to not rise to the argument.

So, for example:

This is nothing personal, but, in line 232 of main.cpp, I've noticed that a try-catch statement is missing, which means exceptions are not being handled. If an exception isn't handled here, function ProcessArray might throw a memory allocation error which will cause the program to crash.

Joey might defensively respond:

But on modern machines a memory allocation error is bound to never occur, so there's no need for a try-catch statement. Besides, I'm doing it for readability!

At which point you sidestep the approaching argument, and simply return to matter-of-fact and a clean exit:

Good programming standards dictate that programs gracefully exit in the event of a major error, as crashing might impact the systems, if any, that rely upon it, especially to not crash. Operating Systems are a prime example of this. You might find a different workaround for this problem.

I find some people hate being told what to do, and will spurn it even if it's the most logical choice. By alerting Joey to the problem, the solution, the reasons for that solution and the fact there might be other solutions, allows Joey to see the problem as an impersonal thing for him to solve (he didn't create the mem alloc error: he has to solve it), he has one known solution, and the freedom to research other unknown solutions.

Joey can either see your solution as the easiest, might devise his own solution (which might be surprising - as a student, he should be learning, not merely copying), or might just choose to ignore it, in which case he suffers the consequences (bonus points if you can create a scenario that causes the error), including any subsequent markdowns on program's performance.

Final trick

If you want to be really hands off, there is one question that will get most people to reconsider a piece or line of code that doesn't require explanation:

What happens if ...

What happens if this ProcessArray hits a mem alloc error?

What happens if this switch statement encounters an unknown case?

etc

Answer 11

There have been many great answers so far, so I'll be brief. The students of mine who have difficulty with accepting criticism are often in a "fixed mindset" rather than a "growth mindset", with terminology from Carol Dweck. Most of my students have been high achievers all their lives. Some have got their self-worth bound up with being "good at" computer science (this is a "fixed" mindset). For those, criticism is interpreted as an attack on their self-worth. The other answers are trying to give ways to phrase criticism so it won't be interpreted in this way. I propose, trying to change the students (switching them to "growth" mindsets), so that they realize that they can always get better, and view constructive criticism as a way to do so. This is obviously a lot more work, but I think it's possibly the most valuable thing we can really teach our students.

Our college president is a big fan of this work of Dweck, so he made a campus-wide push to help students make this switch. They learn about Dweck's work during orientation, and then students are put into "advising circles" that meet once or twice a week for their first semester on campus. The circle has a faculty advisor, and 12 students. It's 1 credit (1/4 of a full course). The advisor can give students things to read, or journaling assignments. So far, it seems to be helping. Our retention rate is up, and there are fewer confrontational students. Generally speaking, if I have a problem student, I think one of the best ways to change his behavior is to put him in a situation where his peers point that he is the only one with a problem. The advising circles help with that.

Incidentally, I think the president initiated this move with the writing program in mind. Many students are resistant to criticism about their writing skills, and humanities professors have been dealing with this forever. I'll bet there is a lot we can learn from them, and I wonder if they also have journals for humanities-educators, where we could read what they've come up with.

Answer 12

Don't actually do it. Often students code with time limits and/or when they are tired. This means that some implementations are done because they're faster to implement or because changing the current implementation would require too much time, or the student didn't even stop to think about other implementations.

With that said, instead of giving a critique, provide a suggestion. Ask what led to that implementation, and (most of the times) they'll realize they could have done differentely in x,y,z. Then, ask what they feel about another implementation and its pros/cons and what could be used.

If the goal is to have a student learn to code, help him get his thinking process in order, not his code.

Answer 13

I don't see why the same practices in the industry wouldn't apply to students. Professionals don't come pre-loaded with firmware to handle code-reviews :) Here's been some of my experiences both giving and receiving code reviews:

1. Prove you're right if telling someone they're wrong. Making a baseless assertion up-front is equivalent to, "I told you so," until proven otherwise.
2. Show how the code will break if you think it will break. Don't just spout rhetoric in favor of different way to do it.
3. Don't add your improvements to the list of changes needed. Keep necessary changes and improvements separate. If the coder likes your improvements, they will implement them.
4. Appeal to their vanity by showing them how you can make their code even better instead of showing all its pitfalls. Learning new tricks is easier than losing bad habits.
5. If you think something is wrong but you can't prove it, ask questions about its use instead of making assumptions. Even a solid educated guess gone-wrong can make a code review less constructive from that point on, and waste your time in the process.

edit:

Emphasis on code-reviews. Gilles brings up some good points when taking into consideration more in-depth criticism of code in the form of code-audits. These steps above are not the steps I would take for a code-audit.

In regards to the main difference between the two and how to interact with people, I would say it has more to do with understanding the core intent of the code and figuring out where it falls short and coming up with more elegant solutions. For example, someone might accidentally re-invent something from C++11's <algorithm> library to help sort something from their class. In a code-review, I would mention this and advise you to remove it and replace it, but in an audit, I would tell you to fix it and remove the redundant code, because of the conversation's scope and the relationship's connotation.

If I'm auditing your code, it's usually because of one of these reasons:

1. It was requested ad-hoc for no specified reason
2. Critical/frequent problems originating from a certain module or modules too often
3. It was scheduled
4. It was requested by the dev

It's usually #4 and if you're playing with some fast and loose code cowboys, #2. So, in both of those scenarios you're basically the mentor/master and less of a peer. You're not there for the thumbs up/down, you're there to take charge and in some cases, teach.

edit 2:

Expanding on code audits and analysis as well as quality, I feel the living legend John Carmack does a good job describing it:

The most important thing I have done as a programmer in recent years is to aggressively pursue static code analysis. Even more valuable than the hundreds of serious bugs I have prevented with it is the change in mindset about the way I view software reliability and code quality.

It is important to say right up front that quality isn't everything, and acknowledging it isn't some sort of moral failing. Value is what you are trying to produce, and quality is only one aspect of it, intermixed with cost, features, and other factors. There have been plenty of hugely successful and highly regarded titles that were filled with bugs and crashed a lot; pursuing a Space Shuttle style code development process for game development would be idiotic. Still, quality does matter.

• Original publication URL: http://altdevblogaday.com/2011/12/24/static-code-analysis/
• Working "reprint" as on 2017: http://www.gamasutra.com/view/news/39328/InDepth_Static_Code_Analysis.php

Answer 14

To borrow from Agile practice, you can adapt a form of the Prime Directive.

Always lead with it before looking at the product/code/process. It can feel repetitive, and even elementary. So it helps to rotate the reader of the quote, and open it to team discussion with new members.

Its value grew and became more appreciated every time a retrospective revolved around a significant incident/bug/crisis. It gets the team past blame, toward the mindset of self-improvement. Before learning the prime directive, I experienced the blame aspect of code reviews consistently.

Edit: Adding the quote here:

Regardless of what we discover, we understand and truly believe that everyone did the best job they could, given what they knew at the time, their skills and abilities, the resources available, and the situation at hand.

The purpose as I understand it is to

1. Create a Safe Zone. Nothing leaves the room. Eliminate fear.
2. Treat the code as a product of the team, not one engineer.
3. Action not attitude. Focus team energy on productive analysis and improvement

Feedback

As mentioned by Ben-I. In a lot of ways, code reviews are feedback (to the team). In feedback, there is an opening to achieve alignment. Thanks for the Feedback p.232, Stone&Heen

The opening is important because it sets the conversation's tone and trajectory.

and further

You're not telling the feedback giver what they can or can't say; you're working to clarify the mutual purpose of the conversation and suggesting a two-way exploration. This helps you get aligned for the rest of the conversation.

Also in feedback, both giving/receiving benefit from practice. In the same way it helps improve your speech delivery, Toastmasters utilize a Evaluator role to practice judging speakers. Have students practice giving, and performing code reviews.