8 ms·
What to Do When Your Colleague Creates Spaghetti Code
- Pica_soO 10y agoYou make interfaces, write everything beyond off as a total failure, and worry about containment and the impending rewrite of his part later. Its like managing a burning forest, you cant extinguish it after a certain dimension is reached, you can only cut the fire from the food and keep it from spreading. The problem is, when your colleague is supposed to work on the same module and you are constantly left debugging stupid stuff out of "working code" he wrote. Management scolds you for being "slow" while the moron gets to play the fast car.
- edw519 10y ago“How can I get Bill to stop writing spaghetti code” is only superficially a technical problem. In reality, it presents a human problem. Actually, it's a management problem. Management must establish and enforce proper process and culture. Necessary but not sufficient: 1. Code standards, determined and agreed upon together 2. Peer review against those standards 3. Requirements Definition policies and procedures 4. Function Specification policies and procedures 5. Unit Testing policies and procedures 6. Technical debt policies and procedures (when to grandfather) None of this has to be that complicated. When the rules are understandable, logical, and everyone agrees to play by them, this takes the monkey off everyone's back. You don't have to worry about approaching Bill about his bad code. You have a "business framework" you can rely upon to take care of that for you. Then no one's the bad guy. This is just good basic I.T. management. Spaghetti code by others is the norm. I have maintained the shitty code of thousands of others. If they're gone, I cuss them all day long. If they're still here, I'm nice. I help management build the processes needed so that I can stay nice. I do my job. I expect management to do theirs.
- asimuvPR 10y agoGood points. Code ends up being a reflection of internal structure. Management is tasked with defining that structure. If it gets to the point where a developer has to fight for good structure then management has failed and there isn't much to do if they can't see any issue.
- nialo 10y agoI agree with this pretty much completely, although I wonder a little how it works at places like my current one, with 4 total programmers. I just wanted to add that probably every 'human problem' or 'people problem' at the individual programmer level is really a management problem. This is the same as how every manufacturing defect is really a process problem.
- ska 10y agoWorks well with small teams. Small teams mostly implies smaller projects & scope, so that scales the documentation nicely. Also, you have fewer communication barriers to defeat.
- csharpminor 10y agoYou're absolutely right, but I'll just note that many, many companies do not have management with the technical acumen to facilitate those decisions. That kind of leadership is a luxury.
- smonff 10y agoBill is also surely a part of management.
- JoeAltmaier 10y agoThat's a big-company solution? When its just a room full of guys, maybe a better approach is to hire more carefully. A half-dozen developers need a skeletal process, especially in a startup.
- ryandrake 10y agoConsistent enforcement is key. You can have all the policies and procedures in the world but if not everyone is motivated to follow them all the time, they're pointless. Also required: the ability to stick to the policies and procedures in the face of "Just ignore the rules this one time--we urgently have to ship this month!!" Cave once, and suddenly every month becomes "urgent".
- CoolGuySteve 10y agoThe term "spaghetti code" comes from a time when programming languages where far less structured. Older languages allowed you to jump anywhere in the program. Modern languages don't let you jump outside of the function scope and have control constructs like "while". I'd argue the modern equivalent is the overuse of design pattern abstraction where every function has one or two lines in it. The symptom is the same: following the control flow of your program requires a maddening amount of jumping around in your editor, making it difficult to reason about. There almost seems to be a cognitive dissonance with some people, where the terms "GOTOs considered harmful" and "spaghetti code" are thrown around but they see nothing wrong with hundreds of tiny functions that are only called from one place. It annoys me that the original point of those terms, that code is for humans, is lost. With regards to the article's example, refactoring your code for unit tests is making life easier for the computer, not humans. And maybe that's why Bill is annoyed, being human and all. And don't quote essays you didn't read. edit: I removed a line but here was the gist of it: Unit tests are great but many languages don't support them well, especially compiled ones. We need to strive for a language that let's you isolate a module for testing without the type system or linker throwing a fit.
- bubbleRefuge 10y agoInteresting point that I've never heard b4. What about the DRY concept?
- jszymborski 10y agoInteresting... Not knowing this background, I always assumed spaghetti code referred to the opposite in modern code: too many nested blocks. I'm always super afraid of growing mounds of logic, and my refactoring usually abstracts a lot of that logic out. If you use a good IDE like IDEA/Jetbrains, you can ease the pain of those "one-line functions" with "click to navigate" function/variable names
- wyldfire 10y ago> His code is just… ugly… to the point that you nearly detect a physical odor from it. Unit tests don't magically bring quality code with them. But it does tend to make it easier to focus on encapsulation. > "How can we overcome years of inertia, a nasty legacy codebase, ... Yeah -- this is a real challenge. BTW Bill is not the problem so much as the codebase. If Bill refuses to consider unit tests, that's a problem that I think is solveable. > Simply walking over to Bill ... "You have a tendency to write enormous methods... Nothing would make any designer more defensive than saying "you do [bad thing]". Bill would probably confess that he, like everyone else, has contributed a few lines here and there to MegaObject::mega_func(). Your challenge is that Bill is apparently less motivated than you are to address the problem. Focus on the code itself. "Bill, you know mega_func()?" "OMG yeah that is a nightmare to debug." "Well, I've been working on unit tests. It took a while but I found a way to create a unit test to cover MegaObject. Now I can test MegaObject without deploying to the target first." "Hmm..." "Newbie Fred took this test for his first feature, a one-liner fix in mega_func()." "Oh yeah?" "Yeah, and he was able to find and fix issues with his implementation without going to the lab. And then he refactored mega_func() by adding some encapsulation to those middle 70 lines. We only need that bit in FROB mode anyways."
- existencebox 10y agoWhile I absolutely agree with "talk about the code itself" rather than beating around the bush in some manufactured discussion, why not just address the issue head on when is proper to do so, in CR, and build a culture where code critique is both acceptable and clearly ONLY a critique of the code at hand? I like to think that I could highlight mega_func in a cr to our team and go "extremely difficult to ingest this long a function as a human or modularly test it. Refactor would be welcome" and they would say "makes sense" and swap it in the same way as if someone pointed at one of my DB schemas and went "that's going to cause you problems because X and Y and I'm not sure what you were smoking when you decided on your constraints." We're professionals here but we're not perfect, and the faster we come to a shared understanding that this is fine, and it's part of professionalism to have each other's backs, the faster I've found my teams able to broach these issues without insult.
- mi100hael 10y ago
- UK-AL 10y agoMake peer code reviews a normal part of the process.
- kelvin0 10y agoGreat advice, and there are many advantages to doing so. However, my experience with that tends to demonstrate that programmers will nit pick at lesser details (function names, coding convention) and not really take the time to understand what the code on review wants to achieve. Of course, the latter is time consuming ... any thoughts on this?
- leekh 10y agoHaving a standards guide or a robo-linters should remove a large chunk of it. Deferring to a document is much easier then arguing.
- kelvin0 10y agoThe nit picking I refer to is not about arguing, once a non-standard piece of code is pointed out, the reviewee usually agrees to correct the issue. I refer to the fact that the reviewers will look at the leaves in the tree and not notice the burning inferno and smoke up ahead in the forest...
- MollyR 10y agoI found the only thing that helps is having a strong team lead who identifies this and has the authority to nudge people back on track.
- fao_ 10y agoTell them this? (Or get your manager to tell them)
- dankohn1 10y agoThoroughly agree that this is a human problem, but I think there are some amazing automated fixes for it, by incorporating code checkers into the continuous integration pipeline. Take a look at how the CII best practices badgeapp [0] uses pronto [1] to run rubocop [2] automatically for free via CircleCI [3] and surface those results via the Github Status API and email. The whole point is to avoid having to manually complain in a code review. Instead, code reviews become a much higher level exercise discussing proper APIs and code structuring, not sphaghetti issues. [0] https://github.com/linuxfoundation/cii-best-practices-badge/commit/9dabce047f9a0ab60988aa39ef6524c7e71c7426#commitcomment-18915455 https://github.com/linuxfoundation/cii-best-practices-badge/... [1] https://github.com/mmozuras/pronto https://github.com/mmozuras/pronto [2] https://github.com/bbatsov/rubocop https://github.com/bbatsov/rubocop [3] https://circleci.com/ https://circleci.com/
- ryandrake 10y agoBut you need to get agreement to implement/work with these automated fixes, and now you're back to it being a human problem. If you're 3rd engineer from the left proposing this, your human problem is convincing your team lead or management to invest in these tools. If you're team lead, your human problem is convincing the rest of the team the tools' value and getting them to buy-in to using them.
- dankohn1 10y agoRight, but as team lead, there's a huge advantage to implementing the tools once, and letting individual developers argue with the robot over coding style (i.e., when they override certain rules), vs. trying to do all code review manually.
- triplesec 10y agoThis is great management, interpersonal, and pragmatic advice for anyone in many conflict situations, not just in code disagreements. Diplomacy, ego-conflict reduction, data-orientation and so many more wise human practices are involved in this.
- smonff 10y agoAlways ask yourself if you don't write vermicelli code before qualifying Bill of spaghetti coder. And don't forget that vermicelli should be used in soup, not with bolognese. Now the question is: will you be able not to write soup code?
- michaelfeathers 10y agoThe thing about unit testing existing code is that it is much easier than most people think: https://michaelfeathers.silvrback.com/characterization-testing https://michaelfeathers.silvrback.com/characterization-testi...
- recursive 10y agoThat doesn't even address the hard part. That link shows how to add tests for a nice pure function. The reason testing existing code is usually hard is that it's inserting records in a database or launching nukes or something, and we're not able to actually do those things in a unit test.
- AnimalMuppet 10y agoOf course not, at least not for the launching nukes. Inserting records in a database? I unit test that all the time. You just need a database on your development machine, rather than accessing the production database.
- recursive 10y agoYou need a database, a way to access it, and some kind of guaranteed initial state, and a way to tell your code to use that stuff instead of the production database. There's no guarantee that some arbitrary existing code base will provide a sane way of providing those things.
- AnimalMuppet 10y agoAn arbitrary existing code base? Perhaps not. The portions I've been working on for any length of time? They will. They'll have unit tests, too. Some kind of guaranteed initial state? Yeah, my tests will set that up every time they run, just like they'll set up everything else they need. Now, if I'm working on Oracle-specific code, say, and I don't have an Oracle license for my desktop, I'm unable to do what I have so bravely stated that I will do. Also, if I'm thrown into a mess with no time to do anything but panic fixes, I don't have time. But never in my career has that been the case for long (maybe I'm fortunate...)
- deleted 10y ago[deleted]
- overcast 10y agoEvery time one of these articles surfaces. I feel like everyone on Hacker News writes immaculate code, in between commenting on new posts.
- deleted 10y ago[deleted]
- DonHopkins 10y agoI'm working on Gorgon Medusa [1] code, which used to be quite beautiful, but now it has snakes for hair and turns you to stone when you look directly at it, so I have to look at its reflection in my shield while I edit it. [1] http://www.greekmythology.com/Myths/Creatures/Medusa/medusa.html http://www.greekmythology.com/Myths/Creatures/Medusa/medusa....
- kelvin0 10y agoThere are 2 extremes in the 'code quality' spectrum: A) the programmer who ships 'stuff that works' but incurs large amounts of technical debt (AKA Bill) B) the metaprogrammer who will only ship anything 'perfect' and will push back on any changes he deems to break his crafted code. Of course, there is a balance to be attained and extreme are rarely the solutions.
- ArkyBeagle 10y agoA middle course is to mock things that will support case B). It won't be perfect, but I think you have to try.
- kelvin0 10y agoNot sure I understand your point, care to elaborate?
- ArkyBeagle 10y agoI think getting the price down on better solutions takes some measure of innovation and thinking. It may be test harnesses. It may be extensive prototyping. It often involves investing in equipment. From the coding perspective, it involves having structure be transparent to function. Everything is explicit; have requirements be represented such that they're almost derivable from the code base without testing. This doesn't mean you don't test; it means you do both. The your concern is the consistency of the written requirements, the code base and the results of tests. As an example, I worked on a thing that would cost tens of thousands for one field test. For about a half or a fourth of the cost of that, I was able to mock a field test in the engineer's desktop - not in a lab, but right there. This required a small amount of equipment, about two-four cubic feet worth. This was based on extensive sets of logged data, translated into models. Of course it's by definition inadequate; there are lies, damn lies and simulations. But being able to anneal defects back through the models means you lose a lot less information. I eventually put a thing together in a VM that mocked the entire system. I actually did feature-adds against that, that worked just fine. I was (briefly) able to show about a factor of ten improvement in defect rate. But nobody on the team wanted to own this. the implicit message was "we do the thing; not that thing you just did." It's now a failing organization, so I'm sure all that got dumpstered. That's fine; I lit a candle instead of cursing the darkness.
- knodi123 10y ago> But you’re about to accuse Bill — Bill! — of writing spaghetti code. Bill, who started with your company when they had carrier pigeons instead of emails, and who singlehandedly built the original version of the software with nothing more than COBOL, baling wire and duct tape. Just who do you think you are, anyway? Ugh, I had this problem once. The guy who wrote our company's web store did it in pure javascript (no libraries) and C (not even c++) on the back end purely for credit card processing. He didn't believe in googling something that he could figure out how to do himself, and consequently he never encountered a single best practice. In the end, he was so resistant to change that our boss had to fire him. He went from being the alpha-dog with a decade of seniority who was treated with utmost respect, to fired, within about two weeks of me volunteering to rewrite the e-store and the CEO accepting my offer. On the one hand, I wish I'd known how to handle that more diplomatically. On the other hand, thank god I don't have to work with such an irritable primadonna. He was the only employee allowed to work from home, because he was deathly allergic to computers (no joke), and had a special setup at his house where the computer was isolated in one room, monitor pressed up against a window, and only a keyboard and mouse in the room with him.
- Slackwise 10y ago> He was the only employee allowed to work from home, because he was deathly allergic to computers (no joke), and had a special setup at his house where the computer was isolated in one room, monitor pressed up against a window, and only a keyboard and mouse in the room with him. Story aside, I'm intrigued by this. Was he allergic to PCB dust or something? Or is this some sort of joke or reference wooshing over my head...?
- douche 10y agoThis sounds like some sort of bizarre psychosomatic thing, like Chuck from Better Call Saul and his "allergy to electricity"
- knodi123 10y agoMy take was it was either that, or a bullshit excuse to allow him to work from home. One the one hand, several other employees assured me his crazy home setup was legit. On the other hand, he would sometimes have to work a couple of days in our office during big events, and he seemed fine.