Slashdot Mirror
Ask Slashdot: How To React To Coworker Who Says My Code Is Bad?
A week ago, you read the other side of the same question. Now, an anonymous reader writes "I have been with my company for 10+ years and have seen many development cycles on our projects. We have a developer intern who has not been on the team for very long. On day one he started ripping into my code on how terrible it is. We have a code base of roughly 50,000 lines of code. When he comes to me with a complaint about the code it is simply because he does not have the experience with it to actually understand what the code is doing. He is a smart guy with lots of promise, he is asking good questions, but how do I get him to look past his own self perceived greatness enough to slow down and learn what we are doing and how we have pulled it off?"
This is an ancient problem, with 10 years experience I'm amazed you haven't run into this constantly throughout your entire career. New guys (even old guys) perceive everything they didn't write as shit.
How you deal with it is dependent on a lot of things.
First: is he right? Maybe your code does suck. Hell maybe you suck! At minimum. code that has been around for a while, has been written by multiple people over a long period of time, been adjusted and re-worked to meet changing requirements, and been done under a deadline usually does suck at least a little. Admitting this is hard.
New guys want to re-write everything and don't understand the value of code maturity... most of the time a re-write isn't practical, and even the shittiest code usually attains remarkable stability by virtue of having all the bugs pounded out through years of use. Reminding him that this isn't a university project and a certain level of ugliness should be expected might help.
If you don't think he's right, learn how to properly describe why you do things the way you did, and conversely expect him to explain why they are wrong. This is the biggest thing to learn when doing code reviews, and applies here. If you can't objectively describe what is wrong using with references to either standard or internal best practices or conventions, arguing code ugliness just becomes subjective. If you want to defend your code, learn how to describe how it doesn't suck.
Having some company guidelines will really help, because it gives you something to point at in defending a decision. Ultimately what one guy considers good code may be considered bad by another. You are always going to have cases where someone thinks your code is too abstract, or not abstract enough, or sacrifices too much performance for maintainability, or too much maintainability for performance. At least with standards, the new devs will rail against the standards rather than personally attack you, and a standards document is a lot easier to defend (and yet still allows good changes without too much politics of hurt feelings).
Critique is only as good as the suggestions for improvement. So, that's your answer. I feel that if someone has issues with my code, then show me better and prove me it is better. In the end, clarity, code reuse, design patterns, performance, all of these things come to play.
Take a step back and seriously consider his criticism, as if he were one of your 10+ year coworkers. Whether or not he's right informs the right reaction.
Might as well close the forum down, this is gonna be the best answer concerning this issue. if only I had mod points
have you seen my sig? there are many others like it but none that are the same
Recently went through this myself. Despite having used a kanban board, used version control, commented code, written unit tests, etc, some junior devs thought the code sucked. My takeaway was that there were still too many barriers to entry. Too many passwords, not enough installation instructions, etc.
Somewhere in the process of learning to write production ready code, it occurred to me that it was necessary to work the process backwards. Begin a project by setting up your hosting or distribution environment before starting to code. Write unit tests before starting to code. And so forth.
Getting other people to contribute to your project requires the same kind of thinking. Set up a project page before you start to code. Write a vision statement before you start to code. Write installation instructions, coding style guidelines, and operations support instructions before you start to code. That way, as you proceed in the project, you have clearly build up the documentation that other people are going to need to join your project. These things shouldn't be started after the fact.
If you can't point a new dev to a website and say 'download the source and instructions' here, it's probably too complicated and will meet resistance.
I'm sure if he re-reads your internal design specifications, coding standards, and comments in the code he will understand your design.
As programmers its an easy trap to fall into thinking that better code always translates into those dollars and cents management seems so hung up on. Sometimes it does, sometimes it doesn't. Yes, some bad managers are too short term focused, but being able to do the math and figure out if the cost of cleaning up some code is going to be justified in the long run is part of a managers role.
Telling management you want to re-write everything is a programmers perogative. Accepting it when the manager comes back and says "what we have now works, our customers arn't complaining, the thing is end of life in 2 years, and even if this made future maintenance free it wouldn't be worth it" is a reality.
As an engineer, I've adopted the maxim that there is no good and bad, only fitness for a particular purpose. I prefer a discussion of trade-offs to statements of principle.
I tend to ask "what requirements does this code fail to meet?" And very often, the reviewer has invented his own new requirement! Depending on your process, your response might be anything from "good point, let's add a test case for that" to "you should submit a Requirements Change Form for that. Make sure to get all the required signatures."
And if the criticism is for something immeasurable like "readability" or "maintainability" you can let your critic make the case to the boss why fixing this code is worth the cost.
[Sir Garlon] is the marvellest knight that is now living, for he destroyeth many good knights, for he goeth invisible.
I've said this at several workplaces:
What the Business values:
1. Correctness
2. Reliability
3. Maintainability
4. Speed
5. Coolness
What the Developer values:
1. Coolness
2. Speed
3. Correctness
4. Maintainability
5. Reliability
Don't believe me? Look at practically ANY open source project. Most are unreliable and impossible to contribute to. They are often incorrect, but they are usually fast and the programmer always thinks what he's doing is mega-cool.
Developers need to adjust their mindset to be valuable to the business, but sadly most business code looks more like the second list than the first.
Peter predicted that you would "deliberately forget" creation 2000 years ago...
This is what a lot of idealistic programmers who are just out of school fail to realize. We should be able to remember that all the code we wrote in college had to be perfect, without any flaws. At the same time, most of these programs were fairly small(
Another problem arises when you have an older developer who is forced to learn new tech where the best practices are drastically different from those he/she is familiar with. For example, if all you've ever done is procedural code(which is what most of my college classes did), and you are tossed into a situation where most of your coding is layered event driven the best practices between the two are so different that you are bound to make mistakes. What I've then see happen in situations like this is that a new developer comes along & sees where there are places that the best practices aren't being followed 100% of the time, and therefore assumes all the code must be crap. Instead, what should happen is the new developer should look at why something was done the way it was & work with the responsible party(not necessarily the original developer) and see if there's a good reason to "fix" the bad code.
Currently I work with a great set of managers. They understand the cost of any project and are very good at prioritizing "fixes". They know that some of the early development doesn't work with the things we are now trying to do, and are will to let us go back & fix things. But they also know that we don't have the resources to fix everything, even things that might reduce the errors & crashes. I have a fellow developer that not fixing everything drives him crazy, and that was one of his first lessons. Nothing's perfect, nothing will be, and our job is do the best we can & worry about the problems only when we are told to.
The boss is interested in the long term effects of having code that doesn't suck, such as lower maintenance time. If the boss wouldn't care when directly told this, that just shows he has bad management skills.
Rewriting is very often the last thing to do with working code. IChucking out working code and reproducing it usually involves relearning all of the reasons those last guys did it that horrible way.
Sometimes I don't even like my own code if I'm coming back to it a year later. It's just the nature of the game. What looks 'good' and 'right' to you changes based on what you know about coding, what problems you've encountered, what you know about the project, what you know about the budget, etc, etc.