November 14, 2011 at 9:15 am
I have to agree with you Phil that humility and tact are very important when dealing with someone else's code. Constructive criticism, properly applied, can work wonders. There is never a good reason to damage a team's morale.
The bottom line is that if the code isn't good enough, then we need to reduce the technical debt and refactor. More importantly, we need to learn from the bad code and pass that knowledge on to the rest of the team. Whenever I fix code of a dubious nature, I always put a comment that explains why the new changes are an improvement.
November 14, 2011 at 9:41 am
majorbloodnock (11/14/2011)
Arguably, though, the quality of the code doesn't matter; it either fulfills its business requirement or it doesn't. If you need to justify downtime to rewrite portions, then why not focus on business benefit? Hardly difficult, then, to show a legacy system needs extensive modification without knocking the fact it's spent years doing what it was designed to do, and it's then irrelevant whether it was a collection of cludges or the epitome of elegance in its prime.
Business benefits like:
It will no longer corrupt your data
It will actually transfer all the data it's supposed to
and
It will do these things in finite time spans
And, no, in this case it has absolutely NOT spent years doing what it was supposed to. It has spent years losing data, corrupting other data, breaking applications, falsifying reports management uses to make business decisions, and costing both IT and line-of-business personnel uncounted man-weeks of excess work and headaches, frustration and conflicts.
Anything I say beyond, "I'm going to work on it because it would be good to do so" will involve listing benefits that are purely a "nicer way to say the existing system sucks".
Imagine saying this about a person, "We have a wonderful method of increasing your productivity and efficiency". Sounds positive and all that, right? But it equally says, "you are not as productive and efficient as we need you to be". The words of the first are "nicer", but both say the same thing, and both are de facto criticism. Sometimes (most often), the nicer way is preferable. But there are times when the "mean" way is what's needed.
In this particular case, prioritizing the issues is helped by stating directly what's wrong with them.
Which helps a manager better in prioritizing projects?:
"Process A loses hundreds of orders per day from customers, sometimes assigns the wrong orders to the wrong customers, and loses parts of the shipping address under common circumstances. It will take 6 weeks to fix, and for at least one of those weeks, the order system will be down completely. Process B makes it so our websites occassionally don't show anything, but only for a few seconds at a time and only during the lowest-traffic hours of the day. It will take 3 weeks to fix, and will involve zero expected downtime."
vs
"Process A can be substantially improved in accuracy, reliability and speed. It will take 6 weeks to refactor for those improvements, and our ordering system will be down for a week during that time. Process B needs some reliability and consistency improvements for off-peak usage. It will take 3 weeks to accomplish those improvements, and will involve zero expected downtime."
The second is substantially more "positive". The first is unquestionably critical of the code (and, indirectly, those who wrote that code). One makes the decision very clear as to which needs attention first, the other obfuscates that urgency.
The first is the EXACT situation I have to deal with.
I am NOT going to powder-puff the problems the code is causing in order to help someone's ego sustain (in my opinion unearned) self-respect, if it will hinder important decision making processes for the managers involved.
Do I go out of my way to criticize? No way. Do I criticize minor issues? Definitely not! Do I criticize someone who has a legitimate reason for not producing code (or other products/services) up to a standard they cannot reasonably be expected to achieve? Of course not.
That's not this situation. This is necessary criticism, in my opinion, and thus I disagree with the premise of "never criticise the code". Since that's the premise of the editorial, I disagree with the editorial. Simple as that.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
November 14, 2011 at 9:43 am
Phil Factor (11/14/2011)
On one memorable occasion, I was strolling around an IT department of an international company with the CTO (IT Director actually) chatting about this and that, when we popped over to look at a dev team and get a quick demo of their work. They were dealing with a legacy system of some antiquity that, nevertheless, worked well and just needed upgrading as the hardware and software got upgraded. One of the programmers happened to say something like 'Jeeze, I'd love to meet the prize idiot that wrote this damned code, and poke him in the eye!' at which the Boss swung around fiercely and said 'You're talking to him, D*** you!'. Oops! (It was true. He had served in many roles for the company's IT department over the years)
If all you're talking about it avoiding undiplomatic statements about the people who wrote the code, I agree with that. But "... prize idiot who wrote ..." isn't criticism of the code, it's criticism of the person. Whole different kettle of fish.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
November 14, 2011 at 10:34 am
GSquared (11/14/2011)
majorbloodnock (11/14/2011)
Arguably, though, the quality of the code doesn't matter; it either fulfills its business requirement or it doesn't. If you need to justify downtime to rewrite portions, then why not focus on business benefit? Hardly difficult, then, to show a legacy system needs extensive modification without knocking the fact it's spent years doing what it was designed to do, and it's then irrelevant whether it was a collection of cludges or the epitome of elegance in its prime.Business benefits like:
It will no longer corrupt your data
It will actually transfer all the data it's supposed to
and
It will do these things in finite time spans
And, no, in this case it has absolutely NOT spent years doing what it was supposed to. It has spent years losing data, corrupting other data, breaking applications, falsifying reports management uses to make business decisions, and costing both IT and line-of-business personnel uncounted man-weeks of excess work and headaches, frustration and conflicts.
As I said, either the code meets the business requirements or it doesn't. If it doesn't, then the business should already be well aware of its shortcomings and so you don't need to criticise; in effect, you're already the good guy.
I did say in one of my previous posts that there are a small number of exceptions I hoped would be self evident. I would suggest a situation as polarised as you're outlining falls into that category. However, even then, you've no need to criticise how the code was written because you can make your point by demonstrating that the process could be improved - exactly as you've said. Don't ask me why, but it seems people are happier to accept being told a process is wrong than that they paid for something that's bad quality.
Semper in excretia, suus solum profundum variat
November 14, 2011 at 10:56 am
It is much better to demonstrate how code can be improved without criticizing code. -- phil
Arguably, though, the quality of the code doesn't matter; it either fulfills its business requirement or it doesn't. If you need to justify downtime to rewrite portions, then why not focus on business benefit?
-- Majorbloodnock
Thanks Phil,
this is a good reminder for me.
It also reminded me of a quote from Dale Carnegie (1936) I've heard more than a few times regarding his Principles For Enhancing Relationships.
. "1.Don’t Criticize, Condemn, Or Complain. Criticizing another person not only damages that person’s reputation, but puts a dent in our own." -- http://www.dcarnegietraining.com/resources/relationship-principles
On the other hand, if the squeaky wheel gets the grease,
How can I apply these principles to effect a change in a software company that consistently overlooks tab order on screens, or uses inconsistent methods for date input on various screens?
-- Optimist with experience and still learning
November 14, 2011 at 11:09 am
majorbloodnock (11/14/2011)
GSquared (11/14/2011)
majorbloodnock (11/14/2011)
Arguably, though, the quality of the code doesn't matter; it either fulfills its business requirement or it doesn't. If you need to justify downtime to rewrite portions, then why not focus on business benefit? Hardly difficult, then, to show a legacy system needs extensive modification without knocking the fact it's spent years doing what it was designed to do, and it's then irrelevant whether it was a collection of cludges or the epitome of elegance in its prime.Business benefits like:
It will no longer corrupt your data
It will actually transfer all the data it's supposed to
and
It will do these things in finite time spans
And, no, in this case it has absolutely NOT spent years doing what it was supposed to. It has spent years losing data, corrupting other data, breaking applications, falsifying reports management uses to make business decisions, and costing both IT and line-of-business personnel uncounted man-weeks of excess work and headaches, frustration and conflicts.
As I said, either the code meets the business requirements or it doesn't. If it doesn't, then the business should already be well aware of its shortcomings and so you don't need to criticise; in effect, you're already the good guy.
I did say in one of my previous posts that there are a small number of exceptions I hoped would be self evident. I would suggest a situation as polarised as you're outlining falls into that category. However, even then, you've no need to criticise how the code was written because you can make your point by demonstrating that the process could be improved - exactly as you've said. Don't ask me why, but it seems people are happier to accept being told a process is wrong than that they paid for something that's bad quality.
The problem isn't that I can't demonstrate "the badness of it" or "the goodness of what will replace it". The problem is that I need to communicate a complex set of situations to management, so that resources can be prioritized based on the severity of each problem, and the expectations of costs in fixing each.
I also couldn't care less about being the good guy.
I'm trying to help managers make informed decisions.
In order to make those, I have to speak to the severity and nature of the known issues with the code. There is no way to do so honestly without criticising the code.
It's not about the actual fixin' it step, it's the talkin' 'bout it step. In discussing the nature of the problem, and the estimations of what will be needed to fix it, there is no way to avoid saying, in effect, "it sucks". I won't, of course, summarize to that depth. I will list details of the symptomology of the process, diagnosis of the underlying disease, as needed, and a recommended course of treatment, to put it slightly medically. But listing out the problems and their relative severity is inherently critical, and totally necessary for the decisions to be informed.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
November 14, 2011 at 11:24 am
Are your ears burning yet from all of us screeching at you for this one?
If I look at the code I wrote 10 years ago I cringe I want to hide it someplace but don't. Bad code is really good code but for a reason other than it is well written. It makes us humble. It makes us grow. It enables us to understand the L-user that is writing the next batch of crappy code and act with compassion.
But, I still get to say, "that sucks!" in so many words. And that is what Paul Randal more-or-less wrote to me when I shared a piece of code I onced used to clear the T-Log in 2K.
We need to acknowledge when it sucks, need to eliminate all pride and emotion from what we write and focus on two things: write great code and help others do the same.
November 14, 2011 at 11:25 am
GSquared (11/14/2011)
majorbloodnock (11/14/2011)
GSquared (11/14/2011)
majorbloodnock (11/14/2011)
Arguably, though, the quality of the code doesn't matter; it either fulfills its business requirement or it doesn't. If you need to justify downtime to rewrite portions, then why not focus on business benefit? Hardly difficult, then, to show a legacy system needs extensive modification without knocking the fact it's spent years doing what it was designed to do, and it's then irrelevant whether it was a collection of cludges or the epitome of elegance in its prime.Business benefits like:
It will no longer corrupt your data
It will actually transfer all the data it's supposed to
and
It will do these things in finite time spans
And, no, in this case it has absolutely NOT spent years doing what it was supposed to. It has spent years losing data, corrupting other data, breaking applications, falsifying reports management uses to make business decisions, and costing both IT and line-of-business personnel uncounted man-weeks of excess work and headaches, frustration and conflicts.
As I said, either the code meets the business requirements or it doesn't. If it doesn't, then the business should already be well aware of its shortcomings and so you don't need to criticise; in effect, you're already the good guy.
I did say in one of my previous posts that there are a small number of exceptions I hoped would be self evident. I would suggest a situation as polarised as you're outlining falls into that category. However, even then, you've no need to criticise how the code was written because you can make your point by demonstrating that the process could be improved - exactly as you've said. Don't ask me why, but it seems people are happier to accept being told a process is wrong than that they paid for something that's bad quality.
The problem isn't that I can't demonstrate "the badness of it" or "the goodness of what will replace it". The problem is that I need to communicate a complex set of situations to management, so that resources can be prioritized based on the severity of each problem, and the expectations of costs in fixing each.
I also couldn't care less about being the good guy.
I'm trying to help managers make informed decisions.
In order to make those, I have to speak to the severity and nature of the known issues with the code. There is no way to do so honestly without criticising the code.
It's not about the actual fixin' it step, it's the talkin' 'bout it step. In discussing the nature of the problem, and the estimations of what will be needed to fix it, there is no way to avoid saying, in effect, "it sucks". I won't, of course, summarize to that depth. I will list details of the symptomology of the process, diagnosis of the underlying disease, as needed, and a recommended course of treatment, to put it slightly medically. But listing out the problems and their relative severity is inherently critical, and totally necessary for the decisions to be informed.
What you're talking about, though, isn't criticism; it's identification of problems and offering of possible solutions. I know that sounds like semantics, but the difference is important.
When I said you were the good guy, I wasn't trying to play to egos. If you criticise someone else's work that you've inherited, someone will sooner or later trot out a paraphrasing of the "workman and his tools" cliche, and the first problem you hit will have people thinking "he can't be that great if he can't fix this problem". However, if you're seen as the good guy and you hit the same problem, people will generally think "it must be a difficult problem if he's having difficulty with it". Same person, same problem, different perspectives. One will get you backing, one won't.
Everything you've outlined is quite right. I agree. However, I don't believe you need to descend into criticism to put your point across, and despite what you think I don't believe you are actually doing that either.
Semper in excretia, suus solum profundum variat
November 14, 2011 at 12:33 pm
majorbloodnock (11/14/2011)
GSquared (11/14/2011)
majorbloodnock (11/14/2011)
GSquared (11/14/2011)
majorbloodnock (11/14/2011)
... LOTSof
text ...
What you're talking about, though, isn't criticism ...
If by "criticism" you mean badmouthing something with statements like "it were writed by a ignoramus" (you'll have to dub in the accent on that one), or "this code sux0rs!", then, yeah, I'm avoiding that.
I'm going by dictionary definition 1: "express disapproval of somebody or something: to express disapproval of or dissatisfaction with somebody or something"
The "badmouthin' it" definition applies more exactly to "disparage": "to refer disapprovingly or contemptuously to somebody or something"
Perhaps I'm suffering from my usual "hypervocabulary disorder", and drawing too fine or too gross a distinction here. Either way, Voltaire's advice applies.
I'm not being contemptuous of it, hence avoiding disparagement, but I am expressing disapproval and dissatisfaction with it, hence being critical. Does that change the level of agreement/disagreement with my statements?
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
November 14, 2011 at 4:16 pm
I thought this was a great editorial and appreciated the good reminder.
I also think this is a walking advertisement for thoroughly documenting the "why" of code as much as possible. If you explain why you are doing X instead of Y, you can make it a whole lot easier for the future programmer to safely make changes/eliminate the guess-work on what the heck you were thinking.
A nice side-benefit is that by documenting the "why" of your code, you can effectively preempt *some* criticism - even by your future self.
November 14, 2011 at 4:18 pm
I agree, there is no reason to deride a person for writing bad code. Saying something that refers to the person being an idiot or saying you want to stab some person in the eye is just plain strange and really un-professional. Point out what is wrong and how it can be fixed and leave it at that.
November 14, 2011 at 6:47 pm
I find myself somewhat confused here. I agree with Phil, and I agree with Gus; but they appear to think they disagree with each other.
And the Major B joins in; I can't work out whether I agree with him or not - but I suspect I probably do, although his line is far less intelligible to me than is Gus' line.
But I think something is getting lost here: part of my job has been to teach; part of that has been to act as formal mentor as people prepare for professional qualification. If I am someone's mentor, I surely have a duty to tell them they have got it wrong? Or can I spend three years saying "have you looked at this alternative approach" instead of "that's just plain wrong" and then, when they need my backing to become IEng or CEng or whatever (in the USA, that would be PEng, but I think you don't have the same concept of mentor) say to the body determining whether they qualify "well, as his mentor I can't recommend him because political correctness never allowed me to tell him when that he was wrong so he never learnt that getting it right mattered"?
So if Phil thinks i shouldn't criticise sharply and clearly in those circumstances, he is (I believe) wrong.
On the other hand, Gus is talking about very different circumstances, where I believe it is usually a good idea to avoid saying someone screwed up; and Gus' description of the software is going to amount to "someone screwed up big time"; so maybe Gus is wrong. On the other hand, Gus is absolutely correct in saying he has to give management an accurate picture on which they can base their decisions, and I don't see how that picture can avoid saying (albeit indirectly) that someone screwed up.
Actually, there could be cases where that's possible; I can imagine saying "this was deliberately designed to be a quick and nasty implementation that would last a couple of years; we are now a couple of years in, and it's conforming to that design aim; but it won't last another two years, so we must now embark on the development of the replacement. Here are the technical details of what is falling apart; it's all as predicted, and the people who did the original softeware should be congratulated on meeting their objectives and documenting what the end state would be so accurately". I can imagine saying it, but I've never said it - if the documentation that would have allowed me to say it ever existed, it's been lost (I suspect it's never existed - I've tried to impose documentation standards that would allow that, and failed - developers and most development project managers don't want to know).
But I've been lucky, and mostly haven't had to criticise any code or any other work (except in my role as a mentor) except my own code when it proved to be wrong. Where I've had legacy code to deal with it's bcause I've been asked to come in and fix a disaster, management have already been fully aware that the code I am going to fix or replace is damaging the business serioudly enough to give me carte blanche (including, sometimes, the ability to recruit my own development team to sort the mess out). If I had inherited things that management didn'y know were crud I might have had to be critical (in the Gus sense) and tell management the truth.
Tom
November 15, 2011 at 3:10 am
There are various times in my career when I should certainly have heeded Phil's advice, enough said. I think I have learnt perhaps the hard way that the only person whose code I should roundly call out is my own. I count this even in the worst circumstances, and like most long term devs I've seen stuff to make the Daily WTF blush. VB4 forms that constructed all form ornamentation on the fly in code with no code indentation or whitespace or comments or procedural breakdown it was just like this sentence very hard to read and understand at all.
In other circumstances I agree it best to look mildly lugubrious and say things like, well, this is working for now, I can see why it ended up like this, but we could take out several hours of administration time per week by ... blah blah blah.
November 15, 2011 at 4:14 am
Perhaps we are thinking of subtly different things when considering how effective the use of criticism is. Of course, we've all met far too many people working in IT who have neither the knowledge nor the aptitude, let alone the sheer practice, to be application developers. However, I wasn't thinking of this when I was arguing against criticizing code out of context, and without considering the constraints under which it was written. Weeding out those who have drifted into the wrong profession is a different matter. An interesting topic, I grant you, but I didn't really intend it to be this one
I'm not even really referring to mentoring developers: The mentoring role is very clearly defined, and within that role, you can certainly give suggestions for improvement, along with the encouragement. A development team is normally poorly structured or equipped for providing mentoring, and I've never liked it when developers have taken it upon themselves to sit in judgement on their peers, and predecessors. I've experienced teams where the alpha males have bullied the less-confrontational developers into their viewpoint on coding, and their partisan view of the technology. I've seen applications disparaged by critical whispering campaigns to the extent that they were scrapped despite proving their worth in testing and being accepted by the users. We've got to have a more scientific way of maintaining standards.
I'd much rather use far better metrics for coding, but with a better tolerance for lateral thinking, where developers can be encouraged to achieve the goals of the application more effectively through unconventional means. How we can do this without relying on the blunt instrument of criticism. The more development work I do, and the more experienced I become, the less certain I am that there is a 'royal road' to perfection in programming practices. As with so many other aspects to technology, it depends....
Best wishes,
Phil Factor
November 15, 2011 at 4:30 am
Let's not forget that Phil has deliberately taken an uncompromising stance to write an editorial that will get us all discussing. He has certainly succeeded.
To clarify my stance, I first and foremost believe it is important to recognise shortcomings so things can be improved. Nowhere in his article has Phil argued against doing this. I also believe the act of criticism dwells on the problem rather than the solution, so should generally be avoided. As a result, I agree with the thrust of Phil's article.
The exceptions to this are when one cannot recognise shortcomings without touching on what's not right with the current situation, because I believe having an accurate picture is the overriding priority. Nonetheless, I do recognise that one's interpretation of the word "criticism" (the dictionary defines it but doesn't mention any implications the word carries) will vary how often one encounters such exceptions.
Even so, even when one cannot avoid some form of criticism, the way it is presented is hugely important, and that brings us back to Phil's summarising statement; humility and tact really are more important habits to acquire than demon coding skills.
Semper in excretia, suus solum profundum variat
Viewing 15 posts - 31 through 45 (of 53 total)
You must be logged in to reply to this topic. Login to reply