InstantForum.NET v2.9.0SQLServerCentralhttp://www.sqlservercentral.com/Forums/notifications@sqlservercentral.comThu, 23 May 2013 13:21:53 GMT20http://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspx[quote][b]L' Eomot Inversé (11/17/2011)[/b][hr][quote][b]sturner (11/16/2011)[/b][hr]I despise poorly written code and the reasons are many:* usually consumes (wastes) precious resources (memory, CPU, I/O)* exhibits poor concurrency* poor scalability* poor documentationThe typical argument I hear for a crappy design as well as crappy impleementation (code) is something like: "we didn't have time to do it right, we had to hurry up and get something into production for the customer.." [i]for the customer.[/i] that must be something like " for the children..."Well of course that contention is merely an excuse for the incompetence of the person(s) that conceived of the design and wrote the code. It really doesn't take any longer to put a well designed product into production that it does to put a POS into production. When you consider all the man-hours spent fixing bugs, applying band-aids and other hacks to make it work and keep it running by countless people its a net loss.That's my opinion and I'm sticking to it.[/quote]I guess I shouldn't say this, but: if that last sentence means that regardless of any evidence that shows up you will stick to your opinion, then your opinion (whether right or wrong) is worthless, because no-one in his right mind would pay the slightest attention to any opinion of yours.If you don't understand that, please go and be a politician and keep well aways from IT and Computing where you could do a lot of harm (of course you could as a politician too, but all politicians do that so you'd make politics no worse).Regardless of that, your opinion that computer memory, cpu and io are precious resources, presumably more precious than people's time and effort, sufficiently precious that something that is bug free, works, and performs well enough to do the job but not much better is by comparisin worthless is so outmoded that it was known to be utterly wrong probably before you touched your first computer (certainly it was well known in 1970 - it had been used in management textbooks as an example of what a successful computer company should never do for a few years by then). Anyone who spends five man years (call it a million dollars) to find a faster algorithm for calculation X which is CPU-bound and takes 5.2 seconds on a 2GHz CPU but needs to be done in 5 seconds dead instead of just upgrading to a more appropriate CPU (extra cost maybe five dollars) is a spendthrift idiot unless he knows he's going to need so many that even on a discounted cash-flow basis he can cover the extra development cost in a reasonable time (remembering to cost in the fact that for a year or so he has to either use the faster chip or not release the product - unless he's found a way of getting babies in less that 9 months by throwing lots of women at the problem - and also remembering to include the "lost opportunity" cost of not using those 5 man years on something a bit more profitable), and any company which makes a habit of that sort of stupidity quickly finds itself in serious financial trouble.[/quote]You are free to express your convoluted opinion and likewise anyone ".. in his right mind" is free to pay attention to it.Thu, 17 Nov 2011 07:02:09 GMTsturnerhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspx[quote][b]sturner (11/17/2011)[/b][hr][quote][b]majorbloodnock (11/17/2011)[/b][hr] That doesn't necessarily excuse the cludge, but it may not be the person who put it in who is ultimately at fault, and I refuse to damn someone for doing their best.[/quote]I don't buy it. Every bad system design and.or implementation I've ever run into could have been avoided or improved upon with little or no extra effort during its inception. One can only assume that the person who did it either did not care or was not competent enough to know any better.A few times when I've had to patch fix/hack an existing system in non-optimal way, I left some comment verbiage in the code as a warning/mea culpa to the poor soul that might have to further modify it down the road.[/quote]But you don't have to buy it. With respect, unless you own the business or you were one of the sponsors of the original implementation, it's not your opinion that counts.And if you do assume the person who did it wasn't yet competent enough to know better, you also have to assume it was a business decision to put them into that position and accept the risk.Thu, 17 Nov 2011 06:51:36 GMTmajorbloodnockhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspx[quote][b]sturner (11/16/2011)[/b][hr]I despise poorly written code and the reasons are many:* usually consumes (wastes) precious resources (memory, CPU, I/O)* exhibits poor concurrency* poor scalability* poor documentationThe typical argument I hear for a crappy design as well as crappy impleementation (code) is something like: "we didn't have time to do it right, we had to hurry up and get something into production for the customer.." [i]for the customer.[/i] that must be something like " for the children..."Well of course that contention is merely an excuse for the incompetence of the person(s) that conceived of the design and wrote the code. It really doesn't take any longer to put a well designed product into production that it does to put a POS into production. When you consider all the man-hours spent fixing bugs, applying band-aids and other hacks to make it work and keep it running by countless people its a net loss.That's my opinion and I'm sticking to it.[/quote]I guess I shouldn't say this, but: if that last sentence means that regardless of any evidence that shows up you will stick to your opinion, then your opinion (whether right or wrong) is worthless, because no-one in his right mind would pay the slightest attention to any opinion of yours.If you don't understand that, please go and be a politician and keep well aways from IT and Computing where you could do a lot of harm (of course you could as a politician too, but all politicians do that so you'd make politics no worse).Regardless of that, your opinion that computer memory, cpu and io are precious resources, presumably more precious than people's time and effort, sufficiently precious that something that is bug free, works, and performs well enough to do the job but not much better is by comparisin worthless is so outmoded that it was known to be utterly wrong probably before you touched your first computer (certainly it was well known in 1970 - it had been used in management textbooks as an example of what a successful computer company should never do for a few years by then). Anyone who spends five man years (call it a million dollars) to find a faster algorithm for calculation X which is CPU-bound and takes 5.2 seconds on a 2GHz CPU but needs to be done in 5 seconds dead instead of just upgrading to a more appropriate CPU (extra cost maybe five dollars) is a spendthrift idiot unless he knows he's going to need so many that even on a discounted cash-flow basis he can cover the extra development cost in a reasonable time (remembering to cost in the fact that for a year or so he has to either use the faster chip or not release the product - unless he's found a way of getting babies in less that 9 months by throwing lots of women at the problem - and also remembering to include the "lost opportunity" cost of not using those 5 man years on something a bit more profitable), and any company which makes a habit of that sort of stupidity quickly finds itself in serious financial trouble.Thu, 17 Nov 2011 06:50:06 GMTL' Eomot Inverséhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspx[quote][b]majorbloodnock (11/17/2011)[/b][hr] That doesn't necessarily excuse the cludge, but it may not be the person who put it in who is ultimately at fault, and I refuse to damn someone for doing their best.[/quote]I don't buy it. Every bad system design and.or implementation I've ever run into could have been avoided or improved upon with little or no extra effort during its inception. One can only assume that the person who did it either did not care or was not competent enough to know any better.A few times when I've had to patch fix/hack an existing system in non-optimal way, I left some comment verbiage in the code as a warning/mea culpa to the poor soul that might have to further modify it down the road.Thu, 17 Nov 2011 06:43:56 GMTsturnerhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspx[quote][b]sturner (11/16/2011)[/b][hr]I despise poorly written code and the reasons are many:* usually consumes (wastes) precious resources (memory, CPU, I/O)* exhibits poor concurrency* poor scalability* poor documentationThe typical argument I hear for a crappy design as well as crappy impleementation (code) is something like: "we didn't have time to do it right, we had to hurry up and get something into production for the customer.." [i]for the customer.[/i] that must be something like " for the children..."Well of course that contention is merely an excuse for the incompetence of the person(s) that conceived of the design and wrote the code. It really doesn't take any longer to put a well designed product into production that it does to put a POS into production. When you consider all the man-hours spent fixing bugs, applying band-aids and other hacks to make it work and keep it running by countless people its a net loss.[/quote]In all this, you're making a fundamental assumption that efficient use of the resources you mentioned is at the top of the list of business priorities in your company.Over the years I've met very few people I've been convinced are happy turning in bad work; most people want to do a good job. If that's true then every cludge I come across has a story behind it to help explain why that was the best that person could do at that time with the time, knowledge and resources then available and within the scope defined at that point. That doesn't necessarily excuse the cludge, but it may not be the person who put it in who is ultimately at fault, and I refuse to damn someone for doing their best.Thu, 17 Nov 2011 02:59:57 GMTmajorbloodnockhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspx[quote][b]sturner (11/16/2011)[/b][hr]I despise poorly written code and the reasons are many:It really doesn't take any longer to put a well designed product into production that it does to put a POS into production. [/quote]Really? I can put a POS into production pretty darn fast. But truly understanding the requirements and capturing them properly in the design while maintaining low coupling, high cohesion and optimal performance while remaining scalable is another matter altogether and takes much longer.My current work environment brings many tools to bear to make sure we do not develop bad designs or write bad code. We use these tools religously and they serve us well: rapid prototyping, TDD, CI and Agile (specifically SCRUM). We are not perfect with some of these tools but we try to improve week to week, month to month and year to year.Anyways, I get the point (I'm quoting Mary Poppendieck here):Build the Right ThingBuild the Thing RightWed, 16 Nov 2011 14:20:05 GMTj_e_ohttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspxI despise poorly written code and the reasons are many:* usually consumes (wastes) precious resources (memory, CPU, I/O)* exhibits poor concurrency* poor scalability* poor documentationThe typical argument I hear for a crappy design as well as crappy impleementation (code) is something like: "we didn't have time to do it right, we had to hurry up and get something into production for the customer.." [i]for the customer.[/i] that must be something like " for the children..."Well of course that contention is merely an excuse for the incompetence of the person(s) that conceived of the design and wrote the code. It really doesn't take any longer to put a well designed product into production that it does to put a POS into production. When you consider all the man-hours spent fixing bugs, applying band-aids and other hacks to make it work and keep it running by countless people its a net loss.That's my opinion and I'm sticking to it.Wed, 16 Nov 2011 13:42:23 GMTsturnerhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspx[quote][b]JJ B (11/14/2011)[/b][hr]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.[/quote]Good point. The policy I set for comments and documentation of all database objects and code is "Why, not What". I find it much more useful to know what business rule is being implemented than to have a comment that says "-- Inserting data from table".Tue, 15 Nov 2011 06:19:07 GMTGSquaredhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspx[quote][b]JJ B (11/14/2011)[/b][hr]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.[/quote]I think you hit on two very important points - comments and documentation. Personally, I'm not very good at the documentation piece. I feel that I'm getting better but not up to the level I'd like to be at yet. The commenting piece I've gotten better at by learning the hard way. In school, I always ended up working on solo projects. Even now in my career, most of my projects are completed on my own. The key difference between the me in school and the me now is that now I'm very aware of the fact that at some point, someone else in my group will need to troubleshoot my code and I have the power to make that a little easier for them by adding appropriate comments.I'd also like to add that debugging tools are very useful. I think of this more for the application side, but the point is still valid. When something goes wrong - and we all know it always does - it's very handy to have some type of debug information available so you know where to start looking for the problem.Tue, 15 Nov 2011 05:46:29 GMTLightVaderhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspxLet'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.Tue, 15 Nov 2011 04:30:06 GMTmajorbloodnockhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspxPerhaps 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, [url=http://www.simple-talk.com/community/blogs/philfactor/archive/2006/03/20/610.aspx] you can certainly give suggestions for improvement, along with the encouragement[/url]. 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....Tue, 15 Nov 2011 04:14:31 GMTPhil Factorhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspxThere 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.Tue, 15 Nov 2011 03:10:23 GMTcall.copsehttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspxI 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.Mon, 14 Nov 2011 18:47:55 GMTL' Eomot Inverséhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspxI 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.Mon, 14 Nov 2011 16:18:18 GMTsteveb. http://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspxI 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.Mon, 14 Nov 2011 16:16:44 GMTJJ Bhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspx[quote][b]majorbloodnock (11/14/2011)[/b][hr][quote][b]GSquared (11/14/2011)[/b][hr][quote][b]majorbloodnock (11/14/2011)[/b][hr][quote][b]GSquared (11/14/2011)[/b][hr][quote][b]majorbloodnock (11/14/2011)[/b][hr] ... LOTS[/quote] of [/quote] text ... [/quote][/quote] What you're talking about, though, isn't criticism ...[/quote]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?Mon, 14 Nov 2011 12:33:55 GMTGSquaredhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspx[quote][b]GSquared (11/14/2011)[/b][hr][quote][b]majorbloodnock (11/14/2011)[/b][hr][quote][b]GSquared (11/14/2011)[/b][hr][quote][b]majorbloodnock (11/14/2011)[/b][hr]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.[/quote]Business benefits like: It will no longer corrupt your dataIt will actually transfer all the data it's supposed toandIt will do these things in finite time spansAnd, 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. [/quote]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.[/quote]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 [i]have[/i] 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.[/quote]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.Mon, 14 Nov 2011 11:25:20 GMTmajorbloodnockhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspxAre 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.Mon, 14 Nov 2011 11:24:30 GMTdg81328http://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspx[quote][b]majorbloodnock (11/14/2011)[/b][hr][quote][b]GSquared (11/14/2011)[/b][hr][quote][b]majorbloodnock (11/14/2011)[/b][hr]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.[/quote]Business benefits like: It will no longer corrupt your dataIt will actually transfer all the data it's supposed toandIt will do these things in finite time spansAnd, 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. [/quote]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.[/quote]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 [i]have[/i] 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.Mon, 14 Nov 2011 11:09:45 GMTGSquaredhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspxIt is much better to demonstrate how code can be improved without criticizing code. -- philArguably, 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? -- MajorbloodnockThanks 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-principlesOn 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?Mon, 14 Nov 2011 10:56:13 GMTEd Salvahttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspx[quote][b]GSquared (11/14/2011)[/b][hr][quote][b]majorbloodnock (11/14/2011)[/b][hr]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.[/quote]Business benefits like: It will no longer corrupt your dataIt will actually transfer all the data it's supposed toandIt will do these things in finite time spansAnd, 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. [/quote]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.Mon, 14 Nov 2011 10:34:45 GMTmajorbloodnockhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspx[quote][b]Phil Factor (11/14/2011)[/b][hr]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)[/quote]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.Mon, 14 Nov 2011 09:43:56 GMTGSquaredhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspx[quote][b]majorbloodnock (11/14/2011)[/b][hr]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.[/quote]Business benefits like: It will no longer corrupt your dataIt will actually transfer all the data it's supposed toandIt will do these things in finite time spansAnd, 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.Mon, 14 Nov 2011 09:41:32 GMTGSquaredhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspxI 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.Mon, 14 Nov 2011 09:15:21 GMTj_e_ohttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspxI'll accept code that goes through the mutations of evolution and the bandaid & tourniquet approach to solving problems - if those that have touched code have left comments. I'm wary of rip&replace on squirrelly code because I can't be sure I've captured everything it does unless I understand every part of the as-is before implementing the to-be. The more squirrelly it is the more likely I won't have the luxury of time to fix. However:I consider it a contribution to the mess if we become aware of bad code and do not comment on it. We were there for some reason. We figured out enough to get today's work accomplished. If we don't leave some bread crumbs for the next maintenance developer then we become part of the problem.I've been telling new developers: "The code comments you leave today will save the poor maintenance programmer of tomorrow. You should care deeply about that person; it will likely be you."Mon, 14 Nov 2011 09:13:38 GMTMike Dougherty-384281http://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspxI've seen a lot of code and if someone makes a choice that I would not have, I don't have a problem with that. However, there is bad code out there by adolescent or inadequately trained programmers that deserves to be criticized. In many cases, this may require complete re-writes to handle changed requirements and the customers may not understand why it will take so long for such a "simple" change.Sometimes the code was so horrendous that it was easier (quicker) to go back and get requirements and read hardware manuals than fixing the code. This has usually happened in software applications interfaced with custom hardware where the developer was an engineer or a shop floor supervisor (in one case) rather than a developer.Mon, 14 Nov 2011 08:46:43 GMTJoe Johnson-482549http://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspxI try not to criticize code too much. I know that I'm still early in my career and my ways are definitely not always the best way. I do criticize code that's badly formatted, though. Maybe it's because formatting was ingrained in my when I first started learning to write code in high school, but I find it very difficult to look at code that isn't consistently formatted. My coworkers laugh when they ask me to look at their code and I reformat it in front of them before I look at. (I've been working with them for almost two years now, I didn't do that when I first started)Mon, 14 Nov 2011 07:18:48 GMTLightVaderhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspxArguably, 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.Mon, 14 Nov 2011 06:57:13 GMTmajorbloodnockhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspx[quote][b]Phil Factor (11/12/2011)[/b][hr]Yes, of course we need to improve the code around us, but it can be done without criticism, in the sense of disparaging the code and the author. As any skilled teacher knows, rewarding and praising good code and giving incentives works well in any Dev team. Criticising code causes havoc, and damages relations. Even when the author has long gone, it isn't an effective way of making progress. It is much better to demonstrate how code can be improved without criticizing code. I know this can be hard to do because intuition often tells you that criticism works, but that's probably because you experienced poor teaching at school and college or Uni. I once wrote an article about positive approaches of changing people's behavior at work, with the catch, or twist, being that I wrote it about the team members turning their manager into a better and happier leader of the team by simple positive techniques.[url=http://www.simple-talk.com/opinion/opinion-pieces/on-training-your-it-manager/]On Training Your IT Manager[/url][/quote]All very true.But when I'm explaining to a manager why I need to take their system offline for a few days to rip its guts out and replace it completely, they want to know why. At that point, I certainly have to review the quality and reliability of the current code, and why incremental, non-interruptive changes aren't the best option in this case.I seem to have two basic options: 1. "Trust me" Tell them I know what I'm doing and why, and expect them to leave it at that. That makes me come across as arrogant and insulting to them.2. "The code I'm replacing stinks and here's why..." Explain in layman's terms why it's better to rip it out and rebuild it. That comes across as criticizing the code, because that's what it is.Of course, there is a third option, which would be don't tell them at all, and just start ripping things out and rebuilding, and let the affected managers and co-workers figure out on their own why there aren't any updates going from system A to system B. That seems even worse. I don't consider that a real option, even, because it seems like a good way to lose a job fast.Even on code that doesn't have to stop functioning for any length of time, and can be incrementally refactored in a way that doesn't interrupt current operations, I have to justify to management why I'm spending my time on that. The company pays me, and the managers charged with the usual fiduciary duties expect to have justification for my salary compared to my projects. They rightfully need to know what I'm working on, and why I pick certain tactical targets. Again, I'm left criticizing the code.Constructive criticism for purposes of educating someone has a place. But not in this setting.Phil, I usually agree with most of what you write. I will sometimes nitpick little bits here and there. Other times I'll play devil's advocate and disagree just to get some action going in the discussion for my own entertainment. (Yes, I really do think that way sometimes.) I do the same to Steve on his editorials. And don't even get me started on politics, where I'll disagree with people just to see what effect it creates. But in this particular case, of never criticizing code, I really do disagree. There are times when it is necessary to do so. Am I likely to offend someone? Possibly. Does that bother me? Yes. Do I consider it the unfortunate consequence of a necessary action? Yes. Surgically removing a tumor inevitably traumatizes healthy tissue and compromises immunology - but a good doctor still recommends and does it to save the patient's life. While taking necessary actions to minimize the trauma, and working dilligently towards less traumatic methodologies, of course.Mon, 14 Nov 2011 06:36:46 GMTGSquaredhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspxI'm critical of all code, my own above of all the rest. But when dealing with all code there are constructive ways to get the improvements accomplished. Mainly I do improvement by suggestion or example. Sometimes a link to an article or a reference to a coworkers or my own similar source for solving a particular problems helps. Yes, I have criticized my boss's and his boss's code. But I had a complete solution to the problem wrapped up in bows and was extremely tactful about the situation. Most of the time they don't know or care about the improvements, just that the problem has been solved without fuss.Mon, 14 Nov 2011 06:35:28 GMTchrisn-585491http://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspxOn 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)Mon, 14 Nov 2011 03:10:12 GMTPhil Factorhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspxGreat article, and on the spot. The key here is to understand that you probably dont know the context in which the code was written. "Just get it done by noon, or its a no go""Of course this system wont handle more than a few hundred customers""Hard code that stuff for now, and we'll get back to it when we've through the acceptance test""Lust mock up a prototype, I promise it wont be put into production""The production system will have a wicked fast SAN, but we wont get much cpu"These kind of statements will affect how the code is written, and none of them will reward good coding practices. And when looked upon in an other context, the code will seem strange, sloppy, meaningless or what not. But in most cases it got the job done in the original context. Then there is the strokes of genius you sometimes find in old code. Starting to look at it, it seems like total gibberish, but in the refactoring process you'll discover elegant trade off's between performance, readability and testability. And then there are blazing fast algorithms, creating heineous but effective data structures and odd processing. I see a good coder as someone that gets the job done according to spec. If the spec will allow bad coding style or ineffective code, so be it. That's a management problem, not a coding problem.Mon, 14 Nov 2011 02:41:07 GMTMr. Phantomblothttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspxI have a rule of thumb that has never let me down. Praise in public, criticise in private and make the apology as public as the sin that prompted it.Phil is quite right to advise a programmer against criticising the code they've inherited except in certain specific instances that I would have hoped were obvious (such as making your boss aware of the quality of what's in place so he or she can factor it into their future decisions). Recognise the code's shortcomings by all means, but then either do something about it or keep quiet.Perhaps the most insidious problem of criticism is the effect on the criticiser. I agree with other posters here that constructive criticism can be very positive, but even then one still has to be careful. When faced with an awkward situation - in this case questionable code - you have a choice. Either you can think, "What a mess; how can I be expected to work with this?" or you can take the view that "There's loads I can do to improve this." A lot of criticism, even some of the constructive type, tends to focus on the former view - the damage control - rather than seeing the potential, so is almost inherently self defeating. The code's the same, but your attitude towards it can make all the difference.Do you want to do your best or would you be happy just doing "the best you can under the circumstances"? Your choice.Mon, 14 Nov 2011 01:56:15 GMTmajorbloodnockhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspxWhile I mostly do agree, I can not agree to 100%.There is bad code and there is bad code. One udf of 2k rows calling a few more huge functions is not good code. When the functions or sp's has lost vision of their purpose it's not good code. Same for .net etc. When a class or sp trieds to do too much instead of keeping it simple, one method, one class, one responsibility, you know there is going to be issues with it.Mon, 14 Nov 2011 00:37:15 GMTIceDreadhttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspxHaving been writing code for over 30 years, some of it good, some of it less so, I know there would be times that a consultant could justify critising my code, but I also know that unless done at the time the code is written, there is no point in critizing it. If the code does not work, say so, if circumstances have changed, and there is now a better way of achieving an outcome, say so, but never critise the coder. They may have moved on from coding and be a fantasic PM, they may be related to the person talking to you, they may just have been having a bad day when it was written, or they may have been working with code from an even older legacy system, the reasons for bad code are numerous. When I started it was immperative to reuse variables for something else to save the few bytes of memory (and tape) having another one would use-- Today, that looks like bad coding!Sun, 13 Nov 2011 22:06:12 GMTN-148823http://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspxThanks for the article Phil.I agree that bad criticism along the lines of "oh wow! look what this idiot did!" is not helpful and just shows you up as ignorant.There are many reasons why non-optimal code is written in whichever environment - and unfortunately, unless you know the author really well, there is usually no way to know the reasons why it happened.I would say there is definitely one exception to this though : when a mate that you know is competent writes some stinking code - in this case you should go for it - they deserve it and so do you! One other exception: If I answer a question on SSC and my answer is incorrect or less than optimal, I would worry if it was not criticised - after all, that is the best thing about these forums - the group review.Sun, 13 Nov 2011 17:58:39 GMTmister.magoohttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspxHi PhilNice editorial, sort of follows on from your piece "Never offend a captive audience"Personally I am happy to receive the criticism. Anyone interested in criticising my code - please feel free to take a shot. If it is cheap, duck. If constructive, I am grateful for the chance to learn, and take the knowledge so gained to the next task with thanks. Those arrogant enough to think that they were perfect when they started out in IT, I would rather not know; if they can't accommodate the lack of experience in others, I have no time for them. For those who think they are perfect now, good luck. For the rest of us, well I for one know that I still have much to learn. That is why I use SQLServerCentral. That's why I like to see a better way of doing something than the way I that came to mind for me. I enjoy the learning.I rarely get the opportunity to criticise, but my biggest bugbear is a lack of commentary. I really like knowing what is going on...Sun, 13 Nov 2011 15:28:30 GMTChrisP-374390http://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspx[quote][b]bitbucket-25253 (11/13/2011)[/b][hr][quote][b]sherifffruitfly (11/13/2011)[/b][hr]Most idiotic editorial I've ever seen.[/quote]I hope your comment was made as a joke, for it was not ....[/quote]That would be called self fulflling prophecy :hehe:Sun, 13 Nov 2011 10:53:03 GMTNinja's_RGR'ushttp://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspx[quote][b]sherifffruitfly (11/13/2011)[/b][hr]Most idiotic editorial I've ever seen.[/quote]I hope your comment was made as a joke, for it was not ....Sun, 13 Nov 2011 10:45:40 GMTbitbucket-25253http://www.sqlservercentral.com/Forums/Topic1204334-263-1.aspxOuch, it's starting to get ugly here...Sun, 13 Nov 2011 10:15:06 GMTOsbourne Cox