I think the gist of his argument is to not be ashamed of your work in the face of criticism. He's not advocating a lack of passion, just to not let the critics get you down.
One way that my team practices this is we don't allow the use of the word 'you' in Code Reviews. Because ultimately you are not talking about the person who wrote it but an artifact of their work. For example: "Can you explain why you did it this way?" vs "This code is hard to follow.". It's a simple rule we do that helps remove some of the personal nature of critiquing someone's work.
I think this is helpful advice for people who are new to software development. My experience has been that there are essentially an infinite number of things to learn and improve upon, so you will never really do anything perfectly. There is always something to criticize, but it may not be worth the time or money to fix it. To give some examples I've come across:
Critic: Component X could be Y times faster if you did it this way.
What you should ask yourself: Is performance of X actually a problem that your user base cares about or has complained about? If you improved performance of X, would it help your team in the long run?
Critic: Your code for X is hard to read.
What you need to ask yourself: Is this a core component that is going to stick around forever or is it some temporary thing you slapped together that is going to be replaced in the near future? If it is going to stick around, you should definitely refactor to make it readable. Otherwise, not worth the time unless you literally have no other priorities.
Critic: I noticed this new UI component you wrote has a bug.
What you should ask yourself: Is it critical to your company/team's image to have your UI working perfectly or do your users mostly care about other functionality which is higher priority? Does the bug stop anyone from using your software properly or being able to do something they want to do?
> Is this a core component that is going to stick around forever or is it some temporary thing you slapped together that is going to be replaced in the near future?
In general I'm in total agreement with your points, but in practice this one is really hard to do well. The majority of code that did anything useful tends to stay around for a long time, whether intentionally or not."There is nothing more permanent than a temporary fix."
This is why best practices and established patterns are so important for long term productivity. They help bridge the gap between effort and quality and in general work well. They're not panacea, but the exceptional case is a different issue.
Yeah, you're totally right here. Maybe a better way to look at this point is whether the code is in the core or the periphery. Give higher priority to refactoring core stuff since many people will have to digest this, as opposed to some periphery item which is stable and rarely changed (assuming you have readability issues in both core and periphery code). A lot of judgements going on as to which code is most important so probably best to have this discussion with multiple team members.
Regardless, my hope was to offer some rebuttals for bad code critics. Getting defensive at every criticism is not productive. Giving in and trying to fix everything is not a smart way to handle criticisms either since you might just be wasting time and money.
While it is a mistake to interpret criticism of code as criticism of yourself, sometimes people intend to criticize you personally. "Only an idiot would do it this way" is meant to condemn you as well as the code. Yes, people who write such comments are jerks. Usually the best thing to do is ignore the personal part. OTOH, if the pattern continues, somebody somewhere needs to offer some counter-criticism so the behavior can be corrected.
One of the things I liked about Caltech when I was a student there (late 70s, early 80s) is that when someone did or suggested something stupid, people would tell them it was stupid [1]. Everyone recognized that everyone has utterly idiotic ideas now and then, and so being told that an idea of yours was stupid didn't imply that you were any dumber than anyone else there.
A typical conversation would go something like this:
me: [suggests some stupid idea]
friend: that's stupid!
me: Huh? [pause a moment to think about it, realize he is right] Oh, yeah, that was stupid.
...and then we move on. Sometimes the last line bit might instead be
me: Huh? [pause a moment to recheck my idea, and see that it was not stupid] No it isn't!
friend [pauses a moment, realizes that he was hasty] OK, maybe it was not stupid.
...and we then consider my idea.
I find this much nicer than the way it is often done nowadays, where what you have to do is try to get the person to figure out for themselves that it is a bad idea, typically by asking them questions designed to make them think about the area where you have spotted a fatal flaw in the hope that they will eventually see it themselves.
[1] Well, not always. Once someone hacked into my account on the mainframe, found something I was working on that had some stupid bugs...and they fixed them and left. I never found out who did it. On general principle I suspect Walter Bright. :-)
Won't you accomplish the same result by asking questions around the suspected fatal flaw? If it's really a flaw, the suggester is made aware of it. If it isn't a flaw, the asker is given evidence that it's not. It just seems like a different approach to these end result.
I also believe that method promotes hashing it out together. Outright rejection of an idea seems to put the onus back solely on the suggester to defend the idea without having been communicated the reason. That said, communicating the reason for rejection goes a long way towards making both process more equivalent.
I should have mentioned that a blunt "that was stupid" was generally only for people you knew. If it was someone you didn't know well you would not be so blunt.
The nice thing about working with very smart people is that when they have one of their rare very bad ideas, they are smart enough to see it right away as soon as they see you reject the idea (assuming they think that you are reasonably bright too).
For the more common case of bad ideas that are not super bad, then yeah, there probably needs to be some discussion to work out if they need to be rejected, or if the bad parts can be fixed.
If you can keep your ego disentangled from your ideas and from how people respond to your ideas, a group can move past the occasional really bad ideas very quickly, and spent most of its time on the good ideas, or on the not good ideas that have some good things in them that can be saved.
Always followed this rule, 100% pure and simple, since the 70's when it all began for me.
Of course, I failed at various degree's in actuality, but I also had major wins with my 'code is not an identity' general policy.
One of those wins involving the realisation that in fact, code is not me, I am not code, but what code really is, is for the User. If the user can't identify with what the code is doing, it is in fact bad code. But only when the user is involved can one really determine good/bad on any kind of scale ..
The corollary to this, which took me a few more years to shake off, was that building a code edifice may not effect any useful change to the world or produce anything worthy of reuse. It's all very sensitive to the context you wrote it in, the design the code serves, and so on. A lot of value is built on simple automations that you could describe with a few lines of script - in those cases, an already existing design is doing the thing you wanted. The heavy lifting happens only after you need a new design.
14 comments
[ 2.6 ms ] story [ 35.0 ms ] threadUnless you're really passionate about your work. Take this from developers and programming will become just a job.
All the things the author mentioned in this article is just a bunch of statements probably based on their personal experience.
Critic: Component X could be Y times faster if you did it this way.
What you should ask yourself: Is performance of X actually a problem that your user base cares about or has complained about? If you improved performance of X, would it help your team in the long run?
Critic: Your code for X is hard to read.
What you need to ask yourself: Is this a core component that is going to stick around forever or is it some temporary thing you slapped together that is going to be replaced in the near future? If it is going to stick around, you should definitely refactor to make it readable. Otherwise, not worth the time unless you literally have no other priorities.
Critic: I noticed this new UI component you wrote has a bug.
What you should ask yourself: Is it critical to your company/team's image to have your UI working perfectly or do your users mostly care about other functionality which is higher priority? Does the bug stop anyone from using your software properly or being able to do something they want to do?
In general I'm in total agreement with your points, but in practice this one is really hard to do well. The majority of code that did anything useful tends to stay around for a long time, whether intentionally or not."There is nothing more permanent than a temporary fix."
This is why best practices and established patterns are so important for long term productivity. They help bridge the gap between effort and quality and in general work well. They're not panacea, but the exceptional case is a different issue.
Regardless, my hope was to offer some rebuttals for bad code critics. Getting defensive at every criticism is not productive. Giving in and trying to fix everything is not a smart way to handle criticisms either since you might just be wasting time and money.
A typical conversation would go something like this:
me: [suggests some stupid idea]
friend: that's stupid!
me: Huh? [pause a moment to think about it, realize he is right] Oh, yeah, that was stupid.
...and then we move on. Sometimes the last line bit might instead be
me: Huh? [pause a moment to recheck my idea, and see that it was not stupid] No it isn't!
friend [pauses a moment, realizes that he was hasty] OK, maybe it was not stupid.
...and we then consider my idea.
I find this much nicer than the way it is often done nowadays, where what you have to do is try to get the person to figure out for themselves that it is a bad idea, typically by asking them questions designed to make them think about the area where you have spotted a fatal flaw in the hope that they will eventually see it themselves.
[1] Well, not always. Once someone hacked into my account on the mainframe, found something I was working on that had some stupid bugs...and they fixed them and left. I never found out who did it. On general principle I suspect Walter Bright. :-)
I also believe that method promotes hashing it out together. Outright rejection of an idea seems to put the onus back solely on the suggester to defend the idea without having been communicated the reason. That said, communicating the reason for rejection goes a long way towards making both process more equivalent.
The nice thing about working with very smart people is that when they have one of their rare very bad ideas, they are smart enough to see it right away as soon as they see you reject the idea (assuming they think that you are reasonably bright too).
For the more common case of bad ideas that are not super bad, then yeah, there probably needs to be some discussion to work out if they need to be rejected, or if the bad parts can be fixed.
If you can keep your ego disentangled from your ideas and from how people respond to your ideas, a group can move past the occasional really bad ideas very quickly, and spent most of its time on the good ideas, or on the not good ideas that have some good things in them that can be saved.
Of course, I failed at various degree's in actuality, but I also had major wins with my 'code is not an identity' general policy.
One of those wins involving the realisation that in fact, code is not me, I am not code, but what code really is, is for the User. If the user can't identify with what the code is doing, it is in fact bad code. But only when the user is involved can one really determine good/bad on any kind of scale ..
tl;dr s/code/user/