Slashdot Mirror
Ask Slashdot: How To Handle a Colleague's Sloppy Work?
An anonymous reader writes "I'm working on a new product with one of the more senior guys at our company. To be blunt: his work is sloppy. It works and gets the job done, but it's far from elegant and there are numerous little (some might say trivial) mistakes everywhere. Diagrams that should be spread over five or six pages are crammed onto one, naming is totally inconsistent, arrows point the wrong way (without affecting functionality) and so forth. Much of this is because he is so busy and just wants to get everything out the door. What is the best way to handle this? I spent a lot of time refactoring some of it, but as soon as he makes any changes it needs doing again, and I have my own work to be getting on with. I submit bug reports and feature requests, but they are ignored. I don't want to create bad feelings, as I have to work with him. Am I obsessing over small stuff, or is this kind of internal quality worth worrying about?"
There's getting the job done, and there's getting the job done in a way that can be maintained by someone else in the future. The only way you should let someone continue developing an unmaintainable mess is when there is absolutely no chance it will ever need to be fixed or added to. I have yet to see that happen.
Might you be complaining about this gentleman? http://ask.slashdot.org/story/13/01/03/2255204/ask-slashdot-how-to-react-to-coworker-who-says-my-code-is-bad
Uh-oh. It looks like he also reads slashdot!
It works and gets the job done, but it's far from elegant and there are numerous little (some might say trivial) mistakes everywhere.
If there are functional mistakes, then it shouldn't be working and the best way to complain would be to make bug reports and document your fixes to the code.
However, if it's not buggy and you are refactoring because it's not "elegant" that's much harder to justify or document. Because it kind of sounds like you are complaining about his coding style while he is being productive and writing a ton of less than perfectly styled code that "works and gets the job done".
Don't send him bug reports and feature requests. If he's in charge of something you care about, ask him if he would give it to you (completely!). Then you can fix whatever you want. You will see that there'll be a new guy soon who thinks that your work is sloppy, sending bug reports and feature requests to YOU!
To follow up on the other part of your question (what do I do), here are my suggestions.
Cynical Idealist
Unfortunately, elegant has been re-defined by many a geek as being the shortest possible line of code, regardless of how obtuse it is to read and understand. You can still see people spouting shell scripts or C statements designed solely to show how clever they are, at the expense of readability or maintainability.
Sig Battery depleted. Reverting to safe mode.
VERY few people have the luxury of the time to spend to tweak and get everything perfect and "just so".
Light travels faster than sound. This is why some people appear bright until you hear them speak.........
I like working with people who get the job done, quickly and simply, and focus on functional completeness and minimizing defects. People who I can count on to tell them "here's what it needs to do" and I can know that I'll get something out that does what we need.
I don't like working with people who obsess about every line of code they produce and who worry more about documenting things internally than about getting working code out the door.
Sure, given the choice I prefer clean, maintainable code to shitty, sloppy code. But complaining about diagram quality in internal documentation? Unless you are making components for NASA or MRI machines, I think you're obsessing about things that don't matter that much.
The reason the guy in question is senior to you is because management likes people they can count on to get shit done.
Who said it's solely "hierarchical" seniority? This entire question smacks of a fresh-out-of-college kid (it is May 3... when's graduation?) being absolutely stunned to learn that the real world of software engineering isn't as pretty and glossy and anal-retentively-dotted-i's-and-crossed-t's as he was led to believe it would be in his college classes.
My guess is that the "senior guy" in question is a "senior guy" by virtue of the fact that he's been doing the work for years, and knows how to ship code.
When you're the new guy, it's always better to understand "why" somebody experienced is doing something a particular way before you obsess over changing it. Sounds like the submitter has walked in, doesn't like somebody else's code style, and is now freaking out about it without making any attempt to understand why the senior guy has chosen to work this way.
If it turns out the answer is "the senior guy is a shitty developer and actively harming the company," then you consider how to deal with that. But I'd be far more inclined to believe the answer is "the new guy is a whiny little bitch."
The only way you should let someone continue developing an unmaintainable mess is when there is absolutely no chance it will ever need to be fixed or added to by you.
FTFY
My boss is like that, he abuses cut-and-paste to the exclusion of proper factoring. The code is sometimes comical. He does it for the exact same reason, he wants to move on to testing/fixing/improving, rather than spending a lot of time dreaming-about-how-it-ought-to-be.
Sometimes, maybe 3 or 4 times in the decade that I've worked with him, I have needed to do a major re-factoring just to be able to shoe-horn in a new feature. He is glad I'm here to do that, because he doesn't enjoy re-factoring. I'm glad he's there to do his part, though, because a lot of times he can throw out a big *working* piece of garbage in just a few days, while I would still be arguing with myself about where to start.
The machine works. Therefore, I cannot point to any component that is broken. The machine works.
So I enjoy this, I look at all of the numerous insurmountable customer problems that we have dispatched over the years together, and it is beautiful to me. I love my boss.
That's my advice to you: learn to love this guy, to think of his foolish shortcuts and disorder as unique tools that a unique person uses to solve the problems in front of him. Consider it "local flavor." If you're being hauled up in front of management and they're blaming you for his bugs, that's one thing. But if the machine works, learn to love it. If you can't love it, quit programming and go into a less creative field.
Diagrams that could fit in one page are spread over five or six pages, he's anal over naming convention and minor details like whether arrows point the right way (whether it affects functionality or not) and so forth. He spends lots of time refactoring code instead of making progress and never gets anything out of the door, costing us money. What is the best way to handle this? Whenever I make necessary improvements he goes over my changes with a fine toothcomb, instead of getting on with his own work he spent a lot of time refactoring some of mine. The annoying little tyke then submits bug reports and feature requests, but which my fellow senior peole read with condescending amusement. I don't want to create bad feelings, as I have to work with him. Is he obsessing over small stuff, and should he see the Bigger Picture"
Donte Alistair Anderson Roberts - hi son!
Karma: Chameleon
I've found that those 'internal demos' pretty much always become production systems if they work. There is rarely such a thing as 'throw-away code', so I've stopped writing stuff like there is.
Is it possible - maybe not likely, just possible - that the submitter is in the right? The following is completely fictional and I'm making it up entirely for illustrative purposes. None of it ever really happened. Honest. Ahem.
I transferred from one department in a previous company to another, and was assigned to help a Principal Software Engineer polish up some code for release. When I first looked at the code, I thought maybe I was in over my head. I felt stupid. I couldn't understand a thing. I brought this up to my manager, who chided me for nitpicking about coding styles.
So I slaved away to understand how the project worked and how all the pieces fit together. I'd ask questions like "why are we monkey patching standard library classes to alter their behavior" and be told "it's easier this way". I'd ask why we had 9 levels of inheritance and be told it was to keep the abstractions clean (although I couldn't understand why 7 of those 9 layers only had one child class that derived from them). Why are chunks of server code in the client? Because that's how the design diagram works. Why did we invent our own unit test framework? Because the others weren't good enough for us.
The whole mess came crashing down when our boss asked me to add a data validation check on a certain input. It was a stupidly simple check like "if value < 0: raise ValueError('value must be >= 0')" - that kind of thing. It turned out to be impossible. There was a hell's brew of code that handled errors by returning None, code that returned a string with the error message, and code that used exception handling. There was not one single clean codepath between the API layer and the database model.
My boss had been more concerned with my questions over the weeks, but it wasn't until I showed him the code that he really "got it". He flipped. Meetings were called. VPs were summoned. Design and code reviews were scheduled. The original coder became so frustrated with what he saw as "micromanagement" - things like "unit tests must actually pass before deployment" and "don't monkeypatch everything" - that he quit and I was given ownership of the ball of mess. Since then, I've managed to make reliable, vastly simpler (seriously, I've probably reduced line count by 2/3 and call stack depth by 3/4), and understandable. It no longer uses ML for a templating language. You can read a method's code to see how it works instead of running "git grep methodname" to see if another module patches it at runtime. I'm no longer ashamed to be associated with it.
Sorry, submitter, but I don't have any advice for you. I do have advice for the rest of the readers, though: consider the possibility that the submitter was telling the truth. There are silly code style differences that shouldn't get in the way of getting your work done. You like studly capped method names and your coworker likes_under_scores? The sun will still rise tomorrow. But just because someone is senior doesn't mean that their code isn't a fetid ball of fail.
Dewey, what part of this looks like authorities should be involved?
Certain comments deserve a +10 (or 11 if you prefer). This is one of them. You have hit the nail on the head. I do exactly this with our engineers where I work. They're terrible at writing documentation that other people in the building need to read. Some of the stuff I have seen is absolutely abysmal. What I decided to do was to be the guy that makes everything pretty and easy to digest before official releases for them. Over time a few of them have come to thank me for this, as it saves them a lot of headaches. The net result is that everyone looks good.