On Constructive Criticism

Last week at php|tek 2009, there were numerous sessions on improving your development practices.  There were some focused on better SVN usage, some offering better Object Oriented design practices, some on better Linux-fu, but none of them generated as much discussion – and angst – as “PHP Code Review” (slides here).  Here's the description:

In this workshop, three PHP experts with different software engineering focuses (testing, architecture, and security) will perform an interactive code review together with the audience. Attendees of this lab will learn how experts look at code what good code and bad code looks like, and how to avoid the most common gotchas. They are invited to bring their own code for an anonymous code review for an increased benefit from the workshop.

Seemingly benign, right?  The reaction to the session was something else entirely.

When you offer criticism of a person's work, project, effort, or whatever, you're walking a dangerous line.  Many people can't take criticism.  It's not because they're dumb or do bad work or anything like that… it boils down to the fact that it can be difficult to separate ourselves from our work.

You're not one of them?  Really?

How many times have you thought to yourself: “This code isn't clean enough to release yet” ?

There may be some legitimate reasons to delay releasing code – hard-coded passwords, etc – but quite often this really means “I'll be completely embarassed if anyone sees this code!”  There's nothing fundamentally wrong with that… in fact, it helps keep some of the worst code off the interwebz and hidden in little pockets of nastiness in your organization.  And honestly, I appreciate that.

Unfortunately, once the code is “good enough” for your standards, that doesn't mean it's actually good.  Combine that with the fact that there are some brilliant people out there and it's impossible for it to be “good enough” for everyone.

When you take criticism, you have to learn to separate yourself from your work.

Design decisions are all about tradeoffs.  Depending on the priorities of the situation, your understanding of the situation, and your own knowledge of the options, you may not make the same decision as someone else.  So?  None of those things reflect on your value as a person or even your intelligence.  A lack of education on a concept or experience in a situation is not the same as “dumb”.  Dumb is making the same mistake over and over again and not learning from it…

On the flip side, when you give criticism, you need to separate the criticism from the criticizee.

I'm not sure that's a word but the point is you need to be careful that you're being critical of the decision and not the person.  And make sure there are legitimate – and ideally tangible – reasons why one option is better than another.  If you say “that was a dumb choice, didn't you know about X!?”, be prepared for a heated discussion.  If you say “well, based on X, this might be a better choice”, you can talk about whether X applies and how much it applies and if Y applies or if those don't apply why not.

The discussion changes from a question of the person's competence to the requirements of the situation.  More importantly, if you're going to offer criticism, you have to propose something that would work better.  It's easy to tear someone down or tear their idea down.  It's much more powerful to help them improve it and make it better.

For example, with the unconference last week, there was a great deal of criticism with the scheduling or whatever… but every single person who shared their thoughts, offered an idea they thought would work better.  I sincerely appreciate even the most critical comments as a result.

Regardless of which side you're on – giving or receiving criticism – you have a responsibility:

As the receiver, you need to know whether the person is criticising you or your work…

As the giver, you need to stay focused on the idea not the person… and offer a better alternative.

And to close out the beginning, here is an interview with Sebastian, Stefan, and Arne from after their presentation: