I remember in some intro-level compsci class learning that credit card numbers were checksummed, and writing basic functions to validate those checksums as an exercize. I was young and was still using my "starter" credit card with a whopping limit of $500, so that was all news to me.Alex's company had a problem processing credit cards: they rejected a lot of credit cards as being invalid. The checksum code seemed to be working fine, so what could the problem be? Well, the problem became more obvious when someone's card worked one day, and stopped working the very next day, and they just so happened to be the first and last day of the month.
Alex had the misfortune to work on the kind of application which has forms with gigantic piles of fields, stuffed haphazardly into objects. A single form could easily have fifty or sixty fields for the user to interact with.That leads to C# code like this:
Once upon a time, Ryan's company didn't use a modern logging framework to alert admins when services failed. No, they used everyone's favorite communications format, circa 2005: email. Can't reach the database? Send an email. Unhandled exception? Send an email. Handled exception? Better send an email, just in case. Sometimes they go to admins, sometimes they just go to an inbox used for logging.Let's look at how that worked.
Jenny had been perfectly happy working on a series of projects for her company, before someone said, "Hey, we need you to build a desktop GUI for an existing API."The request wasn't the problem, per se. The API, on the other hand, absolutely was.The application Jenny was working on represented a billing contract for materials consumed at a factory. Essentially, the factory built a bunch of individual parts, and then assembled them into a finished product. They only counted the finished product, but needed to itemize the billing down to not only the raw materials that went into the finished product, the intermediate parts, but also the toilet paper put in the bathrooms. All the costs of operating the factory were derived from the units shipped out.This meant that the contract itself was a fairly complicated tree structure. Jenny's application was meant to both visualize and allow users to edit that tree structure to update the billing contract in sane, and predictable ways, so that it could be reviewed and approved and when the costs of toilet paper went up, those costs could be accurately passed on to the customer.Now, all the contract management was already implemented and lived library that itself called back into a database. Jenny just needed to wire it up to a desktop UI. Part of the requirements were that line items in the tree needed to have a special icon displayed next to them under two conditions: if one of their ancestors in the tree had been changed since the last released contract, or if they child was marked as "inherit from parent".The wrapper library wasn't documented, so Jenny asked the obvious question: "What's the interface for this?"The library team replied with this:
At one point, someone noticed that some financial transactions weren't summing up correctly in the C# application Nancy supported. It didn't require Superman or a Peter Gibbons to figure out why: someone was using floating points for handling dollar amounts.That kicked off a big refactoring project to replace the usage of double types with decimal types. Everything seemed to go well, at least until there was a network hiccup and the application couldn't connect to the database. Let's see if you can figure out what happened:
Today's anonymous submitter has managed to find a way to do date formatting wrong that I don't think I've seen yet. That's always remarkable. Like most such bad code, it checks string lengths and then adds a leading zero, if needed. It's not surprising, but again, it's all in the details:
If you need a unique ID, UUIDs provide a variety of options. It's worth noting that variants 1, 2, and 7 all incorporate a timestamp into the UUID. In the case of variant 7, this has the benefit of making the UUID sortable, which can be convenient in many cases (v1/v2 incorporate a MAC address which means that they're sortable if generated with the same NIC).I bring this up because Dave inherited some code written by a "guru". Said guru was working before UUIDv7 was a standard, but also didn't have any problems that required sortable UUIDs, and thus had no real reason to use timestamp based UUIDs. They just needed some random identifier and, despite using C#, didn't use the UUID functions built in to the framework. No, they instead did this:
Barry rolled into work at 8:30AM to see the project manager waiting at the door, wringing her hands and sweating. She paced a bit while Barry badged in, and then immediately explained the issue:Today was a major release of their new features. This wasn't just a mere software change; the new release was tied to major changes to a new product line- actual widgets rolling off an assembly line right now. And those changes didn't work."I thought we tested this," Barry said."We did! And Stu called in sick today!"Stu was the senior developer on the project, who had written most of the new code."I talked to him for a few minutes, and he's convinced it's a data issue. Something in the metadata or something?""I'll take a look," Barry said.He skipped grabbing a coffee from the carafe and dove straight in.Prior to the recent project, the code had looked something like this:
Legacy systems are hard to change, and even harder to eliminate. You can't simply do nothing though; as technology and user expectations change, you need to find ways to modernize and adapt the legacy system.That's what happened to Alicia's team. They had a gigantic, spaghetti-coded, monolithic application that was well past drinking age and had a front-end to match. Someone decided that they couldn't touch the complex business logic, but what they could do was replace the frontend code by creating an adapter service; the front end would call into this adapter, and the adapter would execute the appropriate methods in the backend.Some clever coder named this "Backend for Frontend" or "BFF".It was not anyone's BFF. For starters, this system didn't actually allow you to just connect a UI to the backend. No, that'd be too easy. This system was actually a UI generator.The way this works is that you feed it a schema file, written in JSON. This file specifies what input elements you want, some hints for layout, what validation you want the UI to perform, and even what CSS classes you want. Then you compile this as part of a gigantic .NET application, and deploy it, and then you can see your new UI.No one likes using it. No one is happy that it exists. Everyone wishes that they could just write frontends like normal people, and not use this awkward schema language.All that is to say, when Alicia's co-worker stood up shortly before lunch, said, "I'm taking off the rest of the day, BFF has broken me," it wasn't particularly shocking to hear- or even the first time that'd happened.Alicia, not heeding the warning inherent in that statement, immediately tracked down that dev's last work, and tried to understand what had been so painful.
Many years ago, JP joined a Ruby project. This was in the heyday of Ruby, when every startup on Earth was using it, and if you weren't building your app on Rails, were you even building an app?Now, Ruby offers a lot of flexibility. One might argue that it offers too much flexibility, especially insofar as it permits "monkey patching": you can always add new methods to an existing class, if you want. Regardless of the technical details, JP and the team saw that massive flexibility and said, "Yes, we should use that. All of it!"As these stories usually go, that was fine- for awhile. Then one day, a test started failing because a class name wasn't defined. That was already odd, but what was even odder is that when they searched through the code, that class name wasn't actually used anywhere. So yes, there was definitely no class with that name, but also, there was no line of code that was trying to instantiate that class. So where was the problem?
Today's anonymous submitter worked for a "large, US-based, e-commerce company." This particular company was, some time back, looking to save money, and like so many companies do, that meant hiring offshore contractors.Now, I want to stress, there's certainly nothing magical about national borders which turns software engineers into incompetents. The reality is simply that contractors never have their client's best interests at heart; they only want to be good enough to complete their contract. This gets multiplied by the contracting firm's desire to maximize their profits by keeping their contractors as booked as possible. And it gets further multiplied by the remoteness and siloing of the interaction, especially across timezones. Often, the customer sends out requirements, and three months later gets a finished feature, with no more contact than that- and it never goes well.All that said, let's look at some SQL Server code. It's long, so we'll take it in chunks.
Do you know what I had forgotten until this morning? That VBScript (and thus, older versions of Visual Basic) don't require you to use parentheses when calling a function. Foo 5 and Foo(5) are the same thing.Of course, why would I remember that? I thankfully haven't touched any of those languages since about... 2012. Which is actually a horrifyingly short time ago, back when I supported classic ASP web apps. Even when I did, I always used parentheses because I wanted my code to be something close to readable.Classic ASP, there's a WTF for you. All the joy of the way PHP mixes markup and code into a single document, but with an arguably worse and weirder language.Which finally, brings us to Josh's code. Josh worked for a traveling exhibition company, and that company had an entirely homebrewed CMS written in classic ASP. Here's a few hundred lines out of their navigation menu.
Singletons is arguably the easiest to understand design pattern, and thus, one of the most frequently implemented design patterns, even- especially- when it isn't necessary. Its simplicity is its weakness.Bartomiej inherited some code which implemented this pattern many, many times. None of them worked quite correctly, and all of them tried to create a singleton a different way.For example, this one:
You know what definitely never changes? Shipping prices. Famously static, despite all economic conditions and the same across all shipping providers. It doesn't matter where you're shipping from, or to, you know exactly what the price will be to ship that package at all times.Wait, what? You don't think that's true? It must be true, because Chris sent us this function, which calculates shipping prices, and it couldn't be wrong, could it?
We talked about singletons a bit last week. That reminded John of a story from the long ago dark ages where we didn't have always accessible mobile Internet access.At the time, John worked for a bank. The bank, as all banks do, wanted to sell mortgages. This often meant sending an agent out to meet with customers face to face, and those agents needed to show the customer what their future would look like with that mortgage- payment calculations, and pretty little graphs about equity and interest.Today, this would be a simple website, but again, reliable Internet access wasn't a thing. So they built a client side application. They tested the heck out of it, and it worked well. Sales agents were happy. Customers were happy. The bank itself was happy.Time passed, as it has a way of doing, and the agents started clamoring for a mobile web version, that they could use on their phones. Now, the first thought was, "Wire it up to the backend!" but the backend they had was a mainframe, and there was a dearth of mainframe developers. And while the mainframe was the source of truth, and the one place where mortgages actually lived, building a mortgage calculator that could do pretty visualizations was far easier- and they already had one.The client app was in .NET, and it was easy enough to wrap the mortgage calculation objects up in a web service. A quick round of testing of the service proved that it worked just as well as the old client app, and everyone was happy - for awhile.Sometimes, agents would run a calculation and get absolute absurd results. Developers, putting in exactly the same values into their test environment wouldn't see the bad output. Testing the errors in production didn't help either- it usually worked just fine. There was a Heisenbug, but how could a simple math calculation that had already been tested and used for years have a Heisenbug?Well, the calculation ran by simulation- it simply iteratively applied payments and interest to generate the entire history of the loan. And as it turns out, because the client application which started this whole thing only ever needed one instance of the calculator, someone had made it a singleton. And in their web environment, this singleton wasn't scoped to a single request, it was a true global object, which meant when simultaneous requests were getting processed, they'd step on each other and throw off the iteration. And testing didn't find it right away, because none of their tests were simulating the effect of multiple simultaneous users.The fix was simple- stop being a singleton, and ensure every request got its own instance. But it's also a good example of misapplication of patterns- there was no need in the client app to enforce uniqueness via the singleton pattern. A calculator that holds state probably shouldn't be a singleton in the first place. [Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!
Mark was debugging some database querying code, and got a bit confused about what it was actually doing. Specifically, it generated a query block like this:
Now, I would argue that the event-driven lifecycle of ASP .Net WebForms is a bad way to design web applications. And it's telling that the model is basically dead; it seems my take is at best lukewarm, if not downright cold.Pete inherited code from Bob, and Bob wrote an ASP .Net WebForm many many ages ago, and it's still the company's main application. Bob may not be with the company, but his presence lingers, both in the code he wrote and the fact that he commented frequently with // bob was hereBob liked to reinvent wheels. Maybe that's why most methods he wrote were at least 500 lines long. He wrote his own localization engine, which doesn't work terribly well. What code he did write, he copy/pasted multiple times.He was fond of this pattern:
When Steve's employer went hunting for a new customer relationship management system (CRM), they had some requirements. A lot of them were around the kind of vendor support they'd get. Their sales team weren't the most technical people, and the company wanted to push as much routine support off to the vendor as possible.But they also needed a system that was extensible. Steve's company had many custom workflows they wanted to be able to execute, and automated marketing messages they wanted to construct, and so wanted a CRM that had an easy to use API."No worries," the vendor sales rep said, "we've had a RESTful API in our system for years. It's well tested and reliable. It's JSON based."The purchasing department ground their way through the purchase order and eventually they started migrating to the new CRM system. And it fell to Steve to start learning the JSON-based, RESTful API."JSON"-based was a more accurate description.For example, an API endpoint might have a schema like:
It takes a lot of time and effort to build a code base that exceeds 100kloc. Rome wasn't built in a day; it just burned down in one.Liza was working in a Python shop. They had a mildly successful product that ran on Linux. The sales team wanted better sales software to help them out, and instead of buying something off the shelf, they hired a C# developer to make something entirely custom.Within a few months, that developer had produced a codebase of 320kloc I say "produced" and not "wrote" because who knows how much of it was copy/pasted, stolen from Stack Overflow, or otherwise not the developer's own work.You have to wonder, how do you get such a large codebase so quickly?
DZ's tech lead is a doctor of computer science, and that doctor loves to write code. But you already know that "PhD" stands for "Piled high and deep", and that's true of the tech lead's clue.For example, in C#:
"Boy, stringly typed data is hard to work with. I wish there were some easier way to work with it!"This, presumably, is what Gary's predecessor said. Followed by, "Wait, I have an idea!"
Sebastian is now maintaining a huge framework which, in his words, "could easily be reduced in size by 50%", especially because many of the methods in it are reinvented wheels that are already provided by .NET and specifically LINQ.For example, if you want the first item in a collection, LINQ lets you call First() or FirstOrDefault() on any collection. The latter option makes handling empty collections easier. But someone decided to reinvent that wheel, and like so many reinvented wheels, it's worse.
For testing networking systems, load simulators are useful: send a bunch of realistic looking traffic and see what happens as you increase the amount of sent traffic. These sorts of simulators often rely on being heavily multithreaded, since one computer can, if pushed, generate a lot of network traffic.Thus, when Jonas inherited a heavily multithreaded system for simulating load, that wasn't a surprise. The surprise was that the developer responsible for it didn't really understand threading in Java. Probably in other languages too, but in this case, Java was what they were using.
Loading times for web pages is one of the key metrics we like to tune. Users will put up with a lot if they feel like they application is responsive. So when Caivs was handed 20MB of PHP and told, "one of the key pages takes like 30-45 seconds to load. Figure out why," it was at least a clear goal.Combing through that gigantic pile of code to try and understand what was happening was an uphill battle. Eventually, Caivs just decided to check the traffic logs while running the application. That highlighted a huge spike in traffic every time the page loaded, and that helped Caivs narrow down exactly where the problem was.
Early in my career, I had the misfortune of doing a lot of Crystal Reports work. Crystal Reports is another one of those tools that lets non-developer, non-database savvy folks craft reports. Which, like so often happens, means that the users dig themselves incredible holes and need professional help to get back out, because at the end of the day, when the root problem is actually complicated, all the helpful GUI tools in the world can't solve it for you.Michael was in a similar position as I was, but for Michael, there was a five alarm fire. It was the end of the month, and a bunch of monthly sales reports needed to be calculated. One of the big things management expected to see was a year-over-year delta on sales, and they got real cranky if the line didn't go up. If they couldn't even see the line, they went into a full on panic and assumed the sales team was floundering and the company was on the verge of collapse.Unfortunately, the report was spitting out an error: "A day number must be between 1 and the number of days in the month."Michael dug in, and found this "delight" inside of a function called one_year_ago:
"Don't use exception handling for normal flow control," is generally good advice. But Andy's lead had a PhD in computer science, and with that kind of education, wasn't about to let good advice or best practices tell them what to do. That's why, when they needed to validate inputs, they wrote code C# like this:
Alexandar sends us some C# date handling code. The best thing one can say is that they didn't reinvent any wheels, but that might be worse, because they used the existing wheels to drive right off a cliff.
Frequently in programming, we can make a tradeoff: use less (or more) CPU in exchange for using more (or less) memory. Lookup tables are a great example: use a big pile of memory to turn complicated calculations into O(1) operations.So, for example, implementing itoa, the C library function for turning an integer into a character array (aka, a string), you could maybe make it more efficient using a lookup table.I say "maybe", because Helen inherited some C code that, well, even if it were more efficient, it doesn't help because it's wrong.Let's start with the lookup table:
Andre has inherited a rather antique ASP .Net WebForms application. It's a large one, with many pages in it, but they all follow a certain pattern. Let's see if you can spot it.
Kate inherited a system where Java code generates JavaScript (by good old fashioned string concatenation) and embeds it into an output template. The Java code was written by someone who didn't fully understand Java, but JavaScript was also a language they didn't understand, and the resulting unholy mess was buggy and difficult to maintain.Why trying to debug the JavaScript, Kate had to dig through the generated code, which led to this little representative line:
Combining Java with lower-level bit manipulations is asking for trouble- not because the language is inadequate to the task, but because so many of the developers who work in Java are so used to working at a high level they might not quite "get" what they need to do.Victor inherited one such project, which used bitmasks and bitwise operations a great deal, based on the network protocol it implemented. Here's how the developers responsible created their bitmasks:
Mark sends us a very simple Java function which has the job of parsing an integer from a string. Now, you might say, "But Java has a built in for that, Integer.parseInt," and have I got good news for you: they actually used it. It's just everything else they did wrong.
Many nations have some form of national identification number, especially around taxes. Argentina is no exception.Their "CUIT" (Clave Unica de Identificacion Tributaria) and "CUIL" (Codigo Unico de Identificacion Laboral) are formatted as "##-########-#".Now, as datasets often don't store things in their canonical representation, Nick's co-worker was given a task: "given a list of numbers, reformat them to look like CUIT/CUIL. That co-worker went off for five days, and produced this Java function.
Nina's team has a new developer on the team. They're not a junior developer, though Nina wishes they could replace this developer with a junior. Inexperience is better than whatever this Java code is.
A recent code-review on a new build pipeline got Sandra's attention (previously). The normally responsible and reliable developer responsible for the commit included this in their Jenkinsfile:
One of the key points of confusion for people unfamiliar with Java is the distinction between true object types, like Integer, and "primitive" types, like int. This is made worse by the collection types, like ArrayList, which needs to hold a true object type, but can't hold a primitive. A generic ArrayList<Integer> is valid, but ArrayList<int> won't compile. Fortunately for everyone, Java automatically "boxes" types- at least since Java 5, way back in 2004- so integerList.add(5) and int n = integerList.get(0) will both work just fine.Somebody should have told that to Alice's co-worker, who spends a lot of code to do some type gymnastics that they shouldn't have:
Eddie's company hired a Highly Paid Consultant to help them retool their systems for a major upgrade. Of course, the HPC needed more and more time, and the project ran later and later and ended up wildly over budget, so the HPC had to be released, and Eddie inherited the code.What followed was a massive crunch to try and hit absolutely hard delivery dates. Management didn't want their team "rewriting" the expensive code they'd already paid for, they just wanted "quick fixes" to get it live. Obviously, the HPC's code must be better than theirs, right?After release, a problem appeared in one of their sales related reports. The point-of-sale report was meant to deliver a report about which items were available at any given retail outlet, in addition to sales figures. Because their business dealt in a high volume of seasonal items, every quarter the list of items was expected to change regularly.The users weren't seeing the new items appear in the report. This didn't make very much sense- it was a report. The data was in the database. The report was driven by a view, also in the database, which clearly was returning the correct values? So the bug must be in the code which generated the report...
We've talked about ASP .Net WebForms in the past. In this style of development, everything was event driven: click a button, and the browser sends an HTTP request to the server which triggers a series of events, including a "Button Click" event, and renders a new page.When ASP .Net launched, one of the "features" was a lazy repaint in browsers which supported it (aka, Internet Explorer), where you'd click the button, the page would render on the server, download, and then the browser would repaint only the changed areas, making it feel more like a desktop application, albeit a laggy one.This model didn't translate super naturally to AJAX style calls, where JavaScript updated only portions of the page. The .Net team added some hooks for it- special "AJAX enabled" controls, as well as helper functions, like __doPostBack, in the UI to generate URLs for "postbacks" to trigger server side execution. A postback is just a POST request with .NET specific state data in the body.All this said, Chris maintains a booking system for a boat rental company. Specifically, he's a developer at a company which the boat rental company hires to maintain their site. The original developer left behind a barnacle covered mess of tangled lines and rotting hull.Let's start with the view ASPX definition:
Dan's co-workers like passing around TDWTF stories, mostly because seeing code worse than what they're writing makes them feel less bad about how often they end up hacking things together.One day, a co-worker told Dan: "Hey, I think I found something for that website with the bad code stories!"Dan's heart sank. He didn't really want to shame any of his co-workers. Fortunately, the source-control history put the blame squarely on someone who didn't work there any more, so he felt better about submitting it.This is another ASP .Net page, and this one made heavy use of GridView elements. GridView controls applied the logic of UI controls to generating a table. They had a page which contained six of these controls, defined like this:
