Signs your code is unmaintainable


  • I survived the hour long Uno hand

    I'm working on a front-page article based on a chat I had with @accalia this morning about warning signs that your codebase may be entirely unmaintainable. I'm sure you lovely folks can dig up some examples of entirely unmaintainable antipatterns for the front page, right? Anyone got anything handy?


  • Discourse touched me in a no-no place

    @Yamikuronue said in Signs your code is unmaintainable:

    I'm sure you lovely folks can dig up some examples of entirely unmaintainable antipatterns for the front page, right? Anyone got anything handy?

    Not so much code as a management that drives the creation of such code on a massive scale. The practice? MUST HAVE MOAR SHINIES! CAN'T FIX STUFF, JUST BOLT MORE STUFF ON TOP!


  • Impossible Mission - B

    inb4 "Just look at any random part of the :disco: 🐎 codebase".



  • @Yamikuronue said in Signs your code is unmaintainable:

    front-page

    Anything made by that is unmaintainable.


    You've ported forward old code from C that had a limitation on method names so everything is 5 letters, but comments took up space on the hard-drive, and no one knows what the actual crap the methods do.

    So everyone just copy-pasta code that works based on speculation.

    seiya()
    seuya()
    senya()
    sefya()

    Pretty sure they do the same thing but take in different parameters....


  • Discourse touched me in a no-no place

    For a bad code practice, the monkeypatching done widely in the Ruby-on-Rails community is a classic example of how something that makes things appear easier on the surface can really shit on long-term maintainability. The core of the issue is that it makes things so massively magical that when things go wrong, it's very hard to figure out ways to fix them in a sensible amount of time/effort. You end up with something that starts out great but progressively converts into codethulhu's pet black hole and nobody can figure how you got there because every step along the way seemed like a great idea at the time.



  • @Yamikuronue Your dependency graph is unreadable if printed on a single piece of 8.5x11 paper.



  • @Yamikuronue Although it's not particularly entertaining, I'll leave this here:

    Michael Feathers said in Working Effectively with Legacy Code:

    In the industry, legacy code is often used as a slang term for difficult-to-change code that we don't understand. But over years of working with teams, helping them get past serious code problems, I've arrived at a different definition.

    To me, legacy code is simply code without tests. I've gotten some grief for this definition. What do tests have to do with whether code is bad? To me, the answer is straightforward, and it is a point that I elaborate throughout the book:

    Code without tests is bad code. It doesn't matter how well written it is; it doesn't matter how pretty or object-oriented or well-encapsulated it is. With tests, we can change the behavior of our code quickly and verifiably. Without them, we really don't know if our code is getting better or worse.


  • FoxDev

    1. first five lines of several class files in an actively maintained part of our codebase:

      // Decompiled with JetBrains decompiler
      // Type: FloorPlanCLR.FloorPlanManagementService.Triggers
      // Assembly: FloorPlanCLR, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null
      // MVID: 55C2F875-6F43-4FDE-A2C6-6CD7F464A43B
      // Assembly location: C:\Users\[DEVMANAGER]\Pictures\FloorPlanCLR.dll
      
    2. We have aproximately eleventy billion CLR methods in our code base.... each an d every one of which breaks every time a new .net patch is made because CLR is SUPER sensitive to the patch level of the .net framework that is installed and will refuse to execute CLR methods when the version of .net that the function was registered with differs from the one it would be running under.

    3. far to many comments where the entirety of the comment is //BUGBUG

    4. comments from 7 different developers over the years that tag their comments with their three letter intials.... except these 7 devs share a common three letter intital so you don't know which of them left the comment.... not that it really matters because none of tem work for us anymore

    5. When trying to produce a dependency map for our database stored procedures and views all automated tools attempted so far have failed.... probably because we have views that select from views that usea function to format data using a CLR method which in turn selects data from the same views that the original view selects data from which...... oww.... my head hurts.

    6. For our flagship product a non technical VP that thought she was the most technically gifted person at the company had fiat level control over the lowest levels of design decisions, and more than once responded to developers saying her decision was wrong for the product by firing them on the spot. thankfully she got shitcanned, but her legacy lives on in the mutilated corpse of a flagship product she left behind.

    7. you cannot replace an aging and failing server because it runs critical business processes that no one at the company has source code for nor can they or the HPCs that wrote the thing in the first place figure out how to move it to a different server.

    8. this is a people problem not a tech problem but still..... :wtf:: about 60% of your sales process is still done in excel to the point where the salespoeple send out contracts before entering the sale into your sales systems so more than once we've had to reject an already issued and customer signed contract because the contract literally could not be fulfilled by our sales order system, a fact our sales order system would have told the salesperson if they just freaking used the system instead of tracking their sales in excel and manually writing contracts!

    9. we have another system that has different versions installed in dev, test, and prod because we lack the licensing to install teh same version in three environments and the vendor refuses to sell us more licenses for the version of the software we have.... we need to upgrade, but we can't because we decompiled their DLLs and made so many changes to them that literally we can't upgrade without trashing out enture workflow..... the vendor knows this, and that's why they won't licnce us for the old version anymore.....

    10. "cute" server names. i'm sorry but i have limited head space.... is carnation our database server or is that rose?! just name the fucking servers sensibly! it's not professional to have "cute" names because i never can fucking remember what server is which, and that old argument about security through layers is shit. having a cutsey name isn't going to hide where your database server is from someone who has compromized the webserver.... the name's right there in the fucking connection strings! GAH!

    11. I could go on, but even these ten make me wish for a whiskey on the rocks, hold the rocks...... and it's only 10AM

    12. all these numbers are the number 1.



  • @clatter Meh... he's getting philosophical. The user base are effective testers, it's just a long drawn out process. (It's a joke guys.... )



  • Having code that was only ever worked on by one specific person. Especially bad if it's an entire library or application.


  • FoxDev

    @blakeyrat said in Signs your code is unmaintainable:

    @Yamikuronue Your dependency graph is unreadable if printed on a single piece of 8.5x11 paper.

    oh. yeah. that is a bad sign.....

    our dependency graph for one product (that we havent had to touch in years thank the goddess) is printed on a 6'x4' sheet of paper, and you still need a magnifying glass to read it.... we stick it on a cork board so we can tack notes to it when we need to figure something out that it touches....

    apparently the CTO has the digital version somewhere but we've never been able to get her to send it to us. when we've asked we either get a visit from fedex with a new printed copy or no response at all.


  • BINNED

    This one bit me in the ass yesterday:

    INSERT INTO table2
    SELECT * FROM table1
    


  • Every step in your long analysis is a different perl script that takes command line arguments, ignores them, and then writes to files based on the input file it's been passed. When you complete your analysis, delete all the intermediate files. Don't use version control. Do two different things by duplicating the script, calling it scriptOld.pl, and making no changes whatsoever. (I checked with diff! Whyyyy). Repeat.

    Surprise! It's literally impossible to find your way through the maze.


  • Discourse touched me in a no-no place

    @accalia said in Signs your code is unmaintainable:

    you cannot replace an aging and failing server because it runs critical business processes that no one at the company has source code for nor can they or the HPCs that wrote the thing in the first place figure out how to move it to a different server.

    That reminds me of a story I was told about our corporate web infrastructure this week. A user had filed a ticket complaining that emails from a form they were hosting somewhere “on the website” were not getting through. It turned out that there were rather a lot of redirects involved — some visible to the browser, some done internally but in the server log, some via a bit of webservice that internally forwarded stuff, at least one by weird screwing around in DNS and our hardware load-balancer — but it ended up on a VM that was no longer being maintained but which happened to still be alive. (The VM wasn't doing anything to do with its original purpose at this point.) That server was hosting a whole load of web-form-to-email gateways, but some of them were no longer working because the certificate used by those form handlers to contact the email gateway had expired and the main email gateway had been tweaked to be stricter about such things in a belated response to all the POODLE stuff. The mails were still “getting sent”, but now they were queueing locally as a fallback. But some of the other handlers were fine because they used a different email gateway that apparently hadn't yet been fixed. And the VM should never have been doing any of this in the first place, but it's easier to reuse an existing system than bring a new VM online. 💫 (Bringing new physical hardware online is easier provided you can find spare power and a network socket. Our bureaucracy knows how to handle hardware.)

    The take home? Sometimes shit fucks up because stuff far away breaks that you had no idea about. And it's definitely unmaintainable except we have to maintain it anyway because any change is a risk and so must be resisted.



  • I'm wondering if you're planning to mention CADT, though that's more a developers anti-pattern, and it doesn't so much make the code unmaintainable as lead the team to abandon code that might or might not have been maintainable but which they didn't want to be bothered with (because it was written by their predecessors on the team and Not Invented Here Syndrome took over).

    Let's see... both spaghetti code and ravioli code are obvious mentions, though spaghetti is mostly (mostly) a thing of the past.

    God Object/Class is also a no-brainer for this.

    Basic things like lack of documentation, out-of-date documentation, and completely-inaccurate-to-begin-with documentation.

    Misuse or inconsistent use of coding style guidelines, or badly designed style guidelines. Hungarian notation deserves special mention, because a) it was a technique meant to overcome a set of technical limitations that really shouldn't apply to most programming environments today (but often do), b) it is still widely used despite being a total mismatch for most of the styles and paradigms in current use, c) it is widely misunderstood and misapplied (even within MS, as seen by the 'Apps Hungarian' vs. 'System Hungarian' schism; the move from semantic warts to structural warts really undermined the idea, not that it was a good one to begin with but it made it much worse), and d) inconsistent use can make even good code nearly unreadable do to the confusion over what the warts mean, and why some parts of the code have them and not others.

    Let's just go down a list of established anti-patterns: cargo culting, stovepipe, action at a distance, boat anchor, hard coding (and soft coding, especially without enough documentation), magic strings, coding by exception, golden hammer, copypasta programming, and premature optimization all apply.


  • Discourse touched me in a no-no place

    @ScholRLEA said in Signs your code is unmaintainable:

    inconsistent use of coding style guidelines

    That's definitely a sign that the canary in the mine has a BO problem…



  • @Yamikuronue

    Your code base is referencing processes to maintain your Asterisk deployments. CC: @Onyx
    🚎


  • Considered Harmful

    Oh god, do we have some horrible code.

    The problem is that I'm almost literally afraid to go into those parts of the codebase to dig some out and share.


  • BINNED

    @izzion I don't get what exactly you're going for... Processes as programs or activities you should do? Because my installs are actually pretty maintainable by the virtue of dialplan never actually changing on install basis, all customizations are done by settings and AGI scripts.

    Though, Asterisk dialplans are usually a definition of unmaintainable by themselves, yes.


  • Considered Harmful

    OK let's see. Our application is multilingual and localized. You can theoretically localize the text of the dropdowns. Except that would break the huge switch-case (if-else chain actually) that lists out every localized version of the display text and keys functionality off it.

    There are parts that depend on the built-in ToString serialization of anonymous objects, so that if you rename or reorder the properties of such an object, the hard-coded substring indices will change and the code will break.

    I could go on like this for days.


  • Considered Harmful

    Did I mention that everything is wired up with reflection? No compile time errors! Just runtime errors, if you change anything. I mean anything; the most innocent of changes (see above: localizing display strings, reordering properties) will break shit.



  • @accalia

    1. We have aproximately eleventy billion CLR methods in our code base.... each an d every one of which breaks every time a new .net patch is made because CLR is SUPER sensitive to the patch level of the .net framework that is installed and will refuse to execute CLR methods when the version of .net that the function was registered with differs from the one it would be running under.
    1. When trying to produce a dependency map for our database stored procedures and views all automated tools attempted so far have failed.... probably because we have views that select from views that usea function to format data using a CLR method which in turn selects data from the same views that the original view selects data from which...... oww.... my head hurts.
    1. we have another system that has different versions installed in dev, test, and prod because we lack the licensing to install teh same version in three environments and the vendor refuses to sell us more licenses for the version of the software we have.... we need to upgrade, but we can't because we decompiled their DLLs and made so many changes to them that literally we can't upgrade without trashing out enture workflow..... the vendor knows this, and that's why they won't licnce us for the old version anymore.....

    WAT

    Oh, girl... RUN!!!!!


  • FoxDev

    @cabrito said in Signs your code is unmaintainable:

    @accalia

    1. We have aproximately eleventy billion CLR methods in our code base.... each an d every one of which breaks every time a new .net patch is made because CLR is SUPER sensitive to the patch level of the .net framework that is installed and will refuse to execute CLR methods when the version of .net that the function was registered with differs from the one it would be running under.
    1. When trying to produce a dependency map for our database stored procedures and views all automated tools attempted so far have failed.... probably because we have views that select from views that usea function to format data using a CLR method which in turn selects data from the same views that the original view selects data from which...... oww.... my head hurts.
    1. we have another system that has different versions installed in dev, test, and prod because we lack the licensing to install teh same version in three environments and the vendor refuses to sell us more licenses for the version of the software we have.... we need to upgrade, but we can't because we decompiled their DLLs and made so many changes to them that literally we can't upgrade without trashing out enture workflow..... the vendor knows this, and that's why they won't licnce us for the old version anymore.....

    WAT

    Oh, girl... RUN!!!!!

    just when i finally got the budget to fix all this shit? and a PM office that supports me, and a manager that's sane?

    hell no! this is going to be too much fun to fix!

    because not only do i have to fix it, i gotta do it without breaking anything as i roll through the systems fixing shit

    SQUEEEEEEEEE!

    so excited!


  • BINNED

    @accalia I knew your concept of "fun" is unorthodox at times, but now you're starting to scare me...



  • Layers upon layers upon layers, with dead layers between. Here's what the APIs for one of our older products looks like:

    http://i.imgur.com/X8Rf6aV.png

    These days, in practice, our users typically only use the lowest C API or the LabVIEW library, meaning everything in between is mostly dead glue, but new features require modifying EVERY SINGLE layer.

    EDIT: Oops, I forgot to draw in the .NET APIs and wrappers, too!


  • BINNED

    @Yamikuronue

    1. You use some non-standard development language or tool because ... who knows?
    2. Mainframe. Bonus points for using IDMS, Cobol or Assembly. Or why not all three?
    3. Let's repeat that: Mainframe.
    4. No distinction between functional, business rules and the implementation
    5. No clear concepts and definitions across applications and sometimes within application. A 'dossier' doesn't mean exactly the same in all application. That would be too logical.
    6. Some people are un-touchable. It doesn't matter if they have an actual positive input to the company unless they decide to leave you can't make them.
    7. People work isolated. Their work is known as x's product. They only know what it does, why and how.
    8. Some people carry both an development and a functional expert tag. Preferably in some really specialized domain ... like bookkeeping.
    9. Your application(s) are required to follow strict governmental guidelines in their output or workings. This means your designs will be made obsolete by some political wind before it is fully implemented.
    10. Your application(s) are health care related. Bonus puts for shoveling everything around through HL7.

    Maybe I should start that rant thread ...



  • @accalia Ah, I can understand the appeal of the challenge. Codethulhu vs accalia.


  • kills Dumbledore

    Why have one data layer when you can have hundreds?
    Discovered the previous developers didn't believe in closing database connections? Here's the 500 connection.Open calls that need a corresponding close adding in.
    OK, that's the ADO.net connections, now you need to find the ado ones, and whatever else was used in the depths of time when the application was originally written in VB5



  • @accalia said in Signs your code is unmaintainable:

    @blakeyrat said in Signs your code is unmaintainable:

    @Yamikuronue Your dependency graph is unreadable if printed on a single piece of 8.5x11 paper.

    oh. yeah. that is a bad sign.....

    our dependency graph for one product (that we havent had to touch in years thank the goddess) is printed on a 6'x4' sheet of paper, and you still need a magnifying glass to read it.... we stick it on a cork board so we can tack notes to it when we need to figure something out that it touches.....

    6'x4'? That's one huge sheet of paper (over 16 times the standard format). Please tell me you meant 6"x4" (' is for feet, " is for inches).



  • I did a lot of monkey patching in JavaScript because it is simply required. Especially in scenarios where ancient versions of IE are still required (everywhere I have ever worked basically).



  • The correct way to do something is modifying X, but you're afraid of breaking it, so you work around it.



  • You could trying mining the Bad Code page on the old Ward's Wiki (which is still around, though no edits have been made in years). There are some real gems of :wtf:ery in there.

    As I mentioned in the 'test no software' story, the lead developer on that project was addicted to both Copypasta Coding and Coding by Exception, so he would have dozens of almost-identical functions, each handling a slightly different use case. When combined they make maintainability go south in a hurry, especially when used with code-behind-the-form as was common in older versions of VB and related tools such as Access - and CBTF encourages exactly that sort of coding, since to consolidate anything you would have to manually hoist the code out of the event handlers and have the handlers call the normalized code. CBTF is inimical to good design, really, and I doubt anyone today really misses it.



  • Anything that doesn't have automated testing for business critical logic. Working in the the gambling industry there are like crazy bets that you need to understand how they work, especially when it comes to pool betting logic, then you have all the regulatory stuff as well.

    I've worked in most of the major bookies now and none of them do automated testing. The code is always horrific, because it is as @groo says "you are to afraid of breaking it, so add another if statement".

    Also any company that will spend years building their own buggy framework instead of contributing to a good open source framework.



  • @antiquarian said in Signs your code is unmaintainable:

    This one bit me in the ass yesterday:

    INSERT INTO table2
    SELECT * FROM table1
    

    I spent a few months a couple years ago fixing the couple hundred stored procedures in our codebase that did that. It got to the point where people were intentionally making tables with extra varchar columns (think UserDefined01, UserDefined02, ... UserDefined20), and every so often, you'd see someone sending a company wide email to ask, "I'm storing order status in UserDefined11 and ship date in UserDefined17 - is anyone else using those columns for anything?"



  • I still say the best reference for this particular metric is Roedy Green's wonderful article (Located at thb and therefore blocked where I work, and also at some university site as a PDF. There are other versions, and they're all bad: the only good one contains the text "quidquid latine dictum sit, altum sonatur")

    I would go so far as to call it an essential piece of reading for any programmer: How to Write Unmaintainable Code communicates through humor important points that are hard to communicate otherwise.



  • @Yamikuronue How's this:

    When I started at my current job, I had to deal with the following:

    1. The only other developer on staff had started two weeks earlier. We had a printed document detailing about 30 different pieces of software, what they do, and where they were running. About 1/3 of the information was wrong thanks to copypasta. Not to mention that we eventually discovered that there were more projects that weren't even mentioned on that document.
    2. There was an instance of TFS 2005 running on a file server which only had source code for the two biggest projects. None of the checkins for those projects had any comments associated with them. As of today, our current version of TFS 2013 has 5 dozen projects, less than 10 of which are brand new since I started here. I still haven't been able to find the source code for some of the items in the document mentioned in #1 above.
    3. Most of the source code I was able to find that was not under source control was located in a directory made from a backup of a previous developer's workspace. The backup was taken 6 months before I started, just before the developer in question had a hard drive failure. I have no guarantee that the source control I found matches what was in production at the time I started.
    4. Despite official policy to the contrary, most code when I started was (and honestly still is) completely uncommented. Not even a hint of what the code is supposed to do.
    5. Reasonable method and variable names? What's wrong with string1 or returnInt on 300 line methods?
    6. Let's copypasta code to 6 different locations instead of finding a way to make it reusable!
    7. Another one that officially goes against policy: inline SQL queries. Our development standard is to use stored procedure because it's a lot easier to track down where a proc is used than to figure out where an in-line SQL statement needs to be modified. When I started, all of our projects had in-line SQL statements everywhere, and trying to track down where changes needed to be done for any given schema changes were a nightmare.

    And that's just the bullshit I can remember.



  • @abarker So, the 'standard' was more of a refactoring wish-list (emphasis on 'wish'), then?


  • Considered Harmful

    I'm thinking of introducing a HereThereBeDragonsAttribute that I can decorate our most brittle classes with to warn maintenance developers.


    Filed under: AbandonHopeYeWhoEnterHereAttribute



  • I should probably also mention Evil Code, though that was for examples of code that was deliberately vile, usually for the purpose of getting revenge or teaching some asshole a lesson.

    And yeah, I'm totes in the shameless self-promotion by mentioning this one, I know.



  • @Groaner Used to work with his guy who couldn't understand why a customer's DBA wouldn't give him alter table permissions to a running application.


  • Considered Harmful

    I remember an old job writing ASP.NET 1.1 apps, that had no source control, and changes were deployed by copying the bin folder directly. I had maybe a year of experience under my belt.

    I was asked to make a change to an app. I looked everywhere for the source code, but couldn't find it. Hey, look, the cs files are on the web server! Lucky break; I pull down the source and make the change. I rebuild, test locally, and redeploy (making a backup first). In a few hours, there's a shitstorm as tons of regressions are reported.

    Turns out, the cs files were only deployed on the very first deployment. Subsequently, only the bin folder was deployed. My changes were based on the cs files in prod.

    At least I had a backup to roll back with.

    Moral: Use SCM. Always, for everything.



  • Auto-generated headers (c++ .h) without any indication, where they are included, that they are auto-generated.



  • @abarker

    There was an instance of TFS 2005 running on a file server which only had source code for the two biggest projects. None of the checkins for those projects had any comments associated with them. As of today, our current version of TFS 2013 has 5 dozen projects, less than 10 of which are brand new since I started here. I still haven't been able to find the source code for some of the items in the document mentioned in #1 above.

    That is nothing.

    We had over 5000 landing pages all with slightly different CSS, HTML and JS with different affiliate codes hardcoded. If those codes weren't right, the wrong people would be credited their percentage from the affiliate link.



  • @lucas1

    In college, a friend of mine bragged that he made a webpage for some government entity, and he charged for each person that had a profile page, because he hard-coded them all.



  • @error In .NET 2.0 and Web-sites and Web-applications.

    WebSites had a App_Code folder where all core code went, so there was no /bin folder. Everything was compiled dynamically and cached at runtime (which is quite funny because that is kinda how ASP.NET Core now works).

    I worked with ASP.NET 2.0 Websites for years and didn't know anything about the Web Application Stuff. So I was rather confused when there were these things called "designer files" and ".dlls" and there usually isn't an App_Code folder in ASP.NET Web Applications.

    I didn't quite get what source control was, so I kept like every production version of the code base in a separate folder on my dropbox ...



  • @ScholRLEA said in Signs your code is unmaintainable:

    @abarker So, the 'standard' was more of a refactoring wish-list (emphasis on 'wish'), then?

    Apparently. I've been able to mark some of the nonconforming projects "obsolete" and bring others into compliance, so it's a lot better than it was 5 years ago.



  • @xaade most of these was made by one young lady that was kinda sort of a front end dev. No source control, nothing.



  • @lucas1 said in Signs your code is unmaintainable:

    We had over 5000 landing pages all with slightly different CSS, HTML and JS with different affiliate codes hardcoded. If those codes weren't right, the wrong people would be credited their percentage from the affiliate link.

    So many better ways to do that …


  • FoxDev

    @Khudzlin i mean six foot by four foot.... and it's as terrible as it sounds.


  • FoxDev

    @Onyx said in Signs your code is unmaintainable:

    @accalia I knew your concept of "fun" is unorthodox at times, but now you're starting to scare me...

    i get to fix things! :-D of course it's fun!


Log in to reply