Thumbnail 1559656
thumbnail
Large (256x256)

Articles

CodeSOD: We'll Hire Better Contractors Next Time, We Promise
Nona writes: "this is the beginning of a 2100 line function."That's bad. Nona didn't send us the entire JavaScript function, but sent us just the three early lines, which definitely raise concerns:
CodeSOD: Good Etiquette
"Here, you're a programmer, take this over. It's business critical."That's what Felicity's boss told her when he pointed her to a network drive containing an Excel spreadsheet. The Excel spreadsheet contained a pile of macros. The person who wrote it had left, and nobody knew how to make it work, but the macros in question were absolutely business vital.Also, it's in French.We'll take this one in chunks. The indentation is as in the original.
Turning Thirty
Eric O worked for a medical device company. The medical device industry moves slowly, relative to other technical industries. Medical science and safety have their own cadence, and at a certain point, iterating faster doesn't matter much.Eric was working on a new feature on a system that had been in use for thirteen years. This new feature interacted with a database which stored information about racks of test tubes, and Eric's tests meant creating several entries for racks of test tubes. And that's when Eric discovered that the database only allowed thirty racks. Add any more, it would just roll right back over to one.This was odd. The database was small- less than 40MB, even in production- and there were automatic tasks to purge old data for compliance purposes. Why a hard limit of thirty?Eric had only been at the company for a year, so he asked one of the more senior team members, Lester. "Oh yeah, that was before my time. You should probably ask Carl."Later that day, Eric happened to bump into Carl around the coffee maker, and asked the question. "Oh, yeah, I do vaguely remember something about that. It was in the requirements for the product. I thought it was weird, but didn't think too much about it. You should probably ask Elise, she's been here like twenty years."Well, now it was getting curious. Eric went over to the "old building", as it was named, the original office for the company on the other side of the parking lot. Most of the offices had moved to the new building a decade earlier, and it mostly served as fabrication and storage, but a few offices remained.Elise was on the third floor, down a poorly lit hallway, sitting in an office with water-stained acoustical tile in its ceiling. "Oh, yeah, I put that into the requirements document. It's funny, I thought it was weird too, but the system you're working on was a replacement for an older system. Our requirements were derived from those. Let me think... Irving worked on that, but he's dead, god rest him. Penny is retired. Oh, you know, Humbert is still around. He didn't work on that, but he worked on some of the systems that came before that. He's upstairs and on the other side of the building."Eric went upstairs and to the other side of the building. The fourth floor had been last remodeled circa 1985, and the ugly industrial paint on the wall was made even uglier by the fact that someone had replaced most of the flourescent tubes with LEDs. Most. The mismatched color temperature started Eric down the path of a headache.Humbert was in an office similar to Elise's. On his desk was a plaque commemerating 40 years of service with the company. Eric asked about the limitation, and Humbert laughed."You're working on the latest version of a product that initially started on an old PDP-11 running MUMPS. I mean, the first versions, anyway. We ran to desktop computers as fast as we could. I wrote a version for DOS in... oh... '86? I knew none of the facilities we worked with had more than ten or fifteen racks of tubes, and I needed somehow to limit the size of the database so it all fit on a single 5 1/4" floppy disk. I picked thirty, because it seemed like a good round number. Honestly, I'm shocked that the limit still exists."So was Eric. There had been several ground-up-rewrites since 1986, before the one Eric maintained had been released thirteen years ago. Each one of them had chosen to maintain the same limitation, without ever considering why it existed. The rule had simply been copied, mindlessly, for 40 years."I'm kind of impressed," Eric said to Humbert, "in a horrified way.""Me too, kid, me too." [Advertisement] Keep all your packages and Docker containers in one place, scan for vulnerabilities, and control who can access different feeds. ProGet installs in minutes and has a powerful free version with a lot of great features that you can upgrade when ready.Learn more.
Representative Line: Comment Overflow
Today, we look at a representative comment, sent to us by Nona. This particular comment was in a pile of code delivered by an offshore team.
CodeSOD: Tune Out the Static
Henrik H (previously) sends us a simple representative C# line:
CodeSOD: The JSON Template
We rip on PHP a lot, but I am willing to admit that the language and ecosystem have evolved over the years. What started as an ugly templating language is now just an ugly regular language.But what happens when you still really want to do things with templates? Allison has inherited a Python-based, WSGI application which rejects any sort of formal routing or basic web development best practices. Their way of routing requests is simply long chains of "if condition then invokeA elif otherCondition then invokeB". Sometimes, those conditions will directly set the MIME type on the HTTP response.They do use a templating library called Mako for generating their responses. They use it for their HTML responses, obviously. They also use it for their JSON responses, generating code like this:
CodeSOD: Lint Brush Off
A few years back, C# added the concept of "primary constructors". Instead of declaring the storage for class members and then initializing them in the constructor, you can annotate the class itself with the required fields, and C# automatically generates a constructor for you. It's all very TypeScript and very Microsoft, and certainly cuts down on some boilerplate.Esben B's team isn't really using them in many places, but they are using a linter which is opinionated about them. So this in-line constructor causes the linter to complain:
CodeSOD: Cancel Catch
"This WTF is in Matlab" almost feels like cheating. At one place I worked, somebody's job was struggling through a mountain of Matlab code and porting it into C. "This Matlab code looks like it was written by an alien," also doesn't really get much traction- all Matlab code looks like it was written by an alien. This falls into the realm of "Researchers use Matlab, researchers may be very smart about their domain, but generally don't know the first thing about writing maintainable code, because that's not their job."But let's take a look at some MatLab Carl W found:
Empty Pockets
If you've seen one developer recounting how their AI agent deleted production, you've seen them all. They're mostly not interesting stories. It's like watching someone speeding through traffic on a motorcycle without a helmet: the eventual tragedy is sad, but it's unsurprising and not an interesting story to tell. It's not even interesting as a warning: the kind of person who speeds on a motorcycle without a helmet isn't doing so because they don't understand the danger. They've just decided it doesn't apply to them.But the founder of PocketOS, Jer, recently shared how- whoopsie!- their AI agent deleted production. There's a lot of ingredients that go into this particular disaster, which I think makes it interesting, because the use of a poorly supervised AI agent is only one ingredient in this absolute trainwreck of a story.PocketOS is a small company that makes software for rental companies to manage reservations. Car rentals are a big customer, but the tool is more general than that. They manage all of their infrastructure via a service called Railway. Railway is a pretty-looking GUI tool for automating your deployments and the target environments.PocketOS also is heavily adopting Cursor wrapping around the Claude model. They've paid big bucks for the top-end model offered. Many of their components, like Railway, offer MCP services so that their LLM can do useful things. They're using the Claude LLM to automate as much as they can.So far, this is all a pretty typical setup. They pointed Claude at their code and gave it a "routine" task, and sent it to work. It toddled through the problem and encountered a credential issue. It "decided" that the fix for this issue was to delete a storage volume and recreate it. It scanned through the code to find a file containing an API key, found it, and then sent a POST request via cURL to delete the volume in question.Jer writes:
CodeSOD: Not for Nullthing
Today's anonymous submitter sends us some code that just makes your mind go... blank when you look at it.
CodeSOD: Please Find, Rewind
As previously discussed, C++ took a surprisingly long time to get a "starts with" function for strings. It took even longer to get a function called "contains". In part, that's simply because string::find solves that problem.Nancy sends us a... different approach to solving this problem.
CodeSOD: Failing to Fail
Russell F (previously) sends us a small one today. It's not just a representative line, it's a representative comment. More than that, it's a true confession. Russell wrote some code, you see, and the logic was confusing. So, a co-worker added a comment to explain what the code was doing:
Representative Line: A Solid Reference
Today's anonymous submitter works for a large company. It's one of those sorts of companies which has piles, and piles, and piles of paperwork and bureaucracy. It also means that much of their portfolio of software is basic CRUD applications. "Here's a database for managing invoices." "Here's a database for managing desk assignments." "Here's a pile of databases which link our legacy applications to our new ERP system."Which brings us to our representative line. It is not a representative line of code, but a representative line of the design specification. This is the design specification for yet another database-driven application.
Representative Line: Underscore Its Unimportance
Frequent submitter Argle (previously), sends us a short little representative line. The good news is that this line of code came across Argle's screen during a code review: it was being removed. The bad news is that it was sitting in the code base for ages.
CodeSOD: Over and Under Reaction
Today's anonymous submitter sends us two blocks. The first is a perfectly normal line of React code:
Representative Line: Dating Backwards
Another representative line, and this one comes from an Excel spreadsheet. But, per Remy's Law of Requirements gathering ("No matter what the requirements doc says, what your users wanted was Excel"), this one was actually written by a developer. A developer who didn't understand how Excel works, but more important, didn't understand how dates worked either.This comes from Ulysse J.
Three Digit Acronyms
JB has a database table that, at first glance, looks like one of those data warehouse tables that exists to make queries performant. You know the sort, the table that contains every date between 1979 and 2050, or every number out to 1,000,000 or something. It looks dumb, but it helps make certain joins and queries performant.The database table is called three_alpha_numerics. It has two columns: digit, which contains three characters, and is_numeric, which is a a single character: 'Y' or 'N'. It looks roughly like this:
CodeSOD: Find a Bar for This One
A depressing quantity of software is what I would call a "data pump". I have some data over here, and I need it over there. Maybe I'm integrating into a legacy app. Or into an ERP. Or into a 3rd party API. At the end of the day, I have data in one place, and I want it in another place.Sally has a Java application written in the Quarkus framework, which has a nightly batch that works to keep a table of Bar entities in sync with a table of Foo entities. (This anonymization comes from Sally) These exist in the same database. There is also a Bar webservice, which provides information about the Bar entities. The workflow, such as it is, is that the software needs to find all of the Foo entities that do not currently have associated Bar entities, and then call the Bar webservice to get the required information to create those Bar entities.Let's see how that works.
CodeSOD: In the Know
Delilah works in a Python shop. Despite Python's "batteries included" design, that doesn't stop people from trying to make their own batteries from potatoes. For example, her co-worker wrote this function:
CodeSOD: Are There Files Yet?
Are there any files to send? That's the question that Chris C's predecessor had. So they asked it. Again. And again. And again.Chris writes:
CodeSOD: What Condition is This
Untodesu sends us this submission, with this comment:
CodeSOD: Blocked the Date
Volodya sends us some bad date handling code in PHP. Which, I know, you're just reaching for the close tab and yawning when you hear that. You've seen it before. But bear with me, this one still has some fun bits to it.
CodeSOD: Coerce the Truth Out of You
Frank suspected something odd when he spotted a use of React's useMemo function in some JavaScript code. Now, there's nothing wrong with using that method, in and of itself. It watches some variables and recalculates a callback if they change for any reason. It's a great tool for when you want to avoid recalculating expensive things over and over again.But in this case, the calculation in question was isAuthorized, which wasn't an expensive calculation; it was just checking if certain values are set. The code looked like this:
CodeSOD: Build Up
If there's one thing that seems to be a constant source of issues, it's people constructing SQL queries through string concatenation. Even if you're using parameters in the query, I'm opposed to handling raw SQL as strings in my programs. My solution is always "use a builder"- an API that constructs a syntax tree that it can then render to SQL as needed. (Yes, a builder, not an ORM, that's a whole other discussion, I'm not dogmatically anti-ORM, but it's a leaky abstraction at best.)Many languages have such a thing, Java included. Lukasz's team was using Java, and they had a rule: "don't do SQL strings, use a builder". Unfortunately for Lukasz's team, their guideline didn't specify what kind of builder.
CodeSOD: Check and Check
Today's anonymous submitter sends us a React view that presents some admin options. Of course, it should only show us those admin options if the user is authorized to do that. So let's see how they implemented it:
CodeSOD: Driven Development
We should always be wary of "(.+)-driven development". Things like test-driven development, or domain-driven development are fine, but they're also frequently approached from a perspective of dogma, which creates its own terrible outcomes.But let's talk about domain-driven development. Without getting too bogged down into the details of the approach, the idea is pretty straightforward: describe you domain model without reference to any lower-level concerns, so you can effectively write your domain logic in an abstract language tuned to your specific needs. In other words, it's just a pretty good practice. DDD offers tools and techniques for doing it, and as stated, can be adopted as a point of dogma instead of technique.Julien joined a team which bragged about their use of DDD. Everything they did followed DDD best practices, they said. The fact that they piled up all sorts of related buzzwords when talking about it should have been a red flag.Here's one of their "domain" classes:
CodeSOD: Delicious Fudge
Stella (previously) sends us a much elided snippet. The original code is several thousand lines contained in a single try block. But the WTF is pretty clear without seeing all of that:
CodeSOD: Dating in Hungarian
A horse can only be so tenderized, but as well established at this point: I don't like Hungarian Notation. Richard G sends us an example of yet more of it, being misused, as well as some bad date handling. That's basically two of the easiest things to complain about, so let's take a look!
CodeSOD: Caught a Mistake
Daniel recently started a new job. His first task was to fetch some data from the database and render it to the user. Easy enough, and there were already wrapper functions around the database to make it easy. He called execute_read, passed it a query, and checked the results.There were no results. But the query definitely should have returned results. What was going on?
CodeSOD: Required Fields
If you want to connect to another system, you need to supply credentials. That's a pretty obvious requirement. We can set aside the whole technical challenge of managing those credentials and the security problems various techniques create, and just focus in on: you must supply some credentials to authenticate.Lisa has inherited a method which connects to another system. It, correctly, will complain if you don't supply parameters for credentials. It will, incorrectly, mislead you about their requirement:
CodeSOD: Weekly Calculated
There's a language out there called "Progress Advanced Business Language" (or "Open Edge Advanced Business Language"). Just hearing that string of words in a sequence tells you you're in for it. It's a verbose, "English-like" programming language. But we're not here to pick on the language.A long time ago, Mirjam had the "pleasure" of working in a Progress ABL environment. At some point, one of the developers had needed to find a date six months prior to the current date. It didn't need to be accurate, and thus said developer littered the code with comments reminding everyone that it didn't need to be that accurate. They arguably spent more time defending the choice to be inaccurate than it would have taken to write code that would have been accurate.Mirjam doesn't have the code anymore, so what we have here is a mix of her remembered pseudocode, Progress syntax, and my attempts to clarify all of it. Let's not worry too much about the language, and instead focus on the logic:
Representative Line: Sort This Out
Today's anonymous submitter has spent a long time toiling through many, many tickets. Their effort has been an attempt to "save" their employer from the disaster left behind by by a highly-paid consultant. As one does, our submitter started with the highest priority tickets with the highest severity. Eventually, they whittled down that list, and had some bandwidth to start looking at the pieces of the code which clearly weren't exploding right now (because there were no tickets), but were likely to explode at some point in the future (creating a storm of tickets).Scanning through the JavaScript, our submitter found a sort function. That was automatically concerning- why was that particular wheel being reinvented?The first line of the sort function was this:
CodeSOD: When False is True
Lillith was integrating some new tools into an existing Ruby on Rails API. The existing API allowed you to send a dry_run flag along with the request, so that you could have the service calculate its changes without applying them.The problem was, the new tool Lillith was integrating could send, in the body of the request, {"dry_run": false}, but the service would see it as true. Consistently.The helper method which checked for "true" parameters looked like this:
CodeSOD: Do a Lot to Do Nothing
Today's anonymous submitter works in finance. I'll let them start the introduction:
CodeSOD: Authorized Logger
Gretchen's company recently got purchased by Initech. Specifically, they were bought for their dev team, of all things. They had a few software products that were high performers, and Initech wanted that secret sauce. They bought the company, and then split the dev team up and migrated the developers to new products.That actually worked out okay for Gretchen, most of the time. For a few projects, the dev team was given some requirements and a free hand to figure out how to deliver them. They were free to reuse code that existed or rewrite entirely, based on their own judgement. They were free to pick the tools they wanted to use, and the results worked out well.But there were some projects that... were a different story. After those successes, Gretchen got moved onto a project that was 90% firefighting. The app had code like this:
The Roadmap
When Gary was called in for a meeting with a few of his managers- because of course he had several- he thought it was going to be for an "attaboy", because things had been going really well for the past few months.Gary had inherited a mess, and taken over a nightmare application. It was the kind of application that should be a simple CRUD-style data-driven app, but somehow despite only having 20ish entities it managed, someone had generated 500+ controllers for managing them. Most of those controllers were copy/pasted code with minor changes in the WHERE clause of a SQL query.And that was just the code. The infrastructure was similarly a mess, with duplicate resources provisioned in their cloud host. There was no CI/CD, no unit tests to speak of, no deployment process that wasn't "manually copy these files and pray". And uptime? You've heard about "five nines", but this product was lucky to get even one nine. Especially because the manual deployment process meant a few hours of downtime.And that was just the infrastructure. The backlog was similarly messy. There were lots of tasks- many thousands- but not a single one had a priority. Most of the tasks were something like, "Fix database timeouts", or "Bug 531" with no description to explain what they were. At best, some of the "new feature" tasks linked to a Google Doc that explained a software roadmap that had been last updated in 2020.So with no guidance, Gary and the rest of his team got to work. Cloud costs were massive. Just cutting the duplicate resources would help, but with actual planning it wasn't hard to find even bigger wins. In total, Gary got the cloud costs down 60%- essentially saving the company a small multiple of his salary every year.With that out of the way, getting a CI/CD pipeline running was next. Within a few weeks, manual deployments were gone. Everything was automated. Downtime nearly vanished. And now, with all the cost savings in cloud resources, for a fraction of what they were paying, it was easy to automate provisioning test environments for each new feature.So Gary was very ready for some congratulations when he sat down with management. He was prepared to discuss all the wins he and the rest of the developers on the project had gotten over the past few months."I'm sure you know why we're sitting down," Manager the First said when they settled into the conference room."I'm sure," said Manager the Second."We have some concerns about your performance," Manager the Third said."My performance?" Gary asked."Yes," said Manager the First. "Let me pull up the backlog.""And the roadmap," said Manager the Second."Yes, I'm getting that up too, thank you." The trio of managers struggled with pulling up the appropriate pages, and after about 15 minutes, gave up. Instead, they discussed their complaint without visual aids. "You haven't completed any of the tasks on the roadmap. Bug 673 has been open since you started on the team. None of the roadmap milestones have been touched. There's absolutely no progress.""Okay, but that document was wildly out of date," Gary said. "Instead I put cycles into solving the actual problems we're having. I've saved the company a huge amount of money. I've gotten our development cycle time down to a fraction of what it was. And we have basically no downtime!""That's all very nice, I'm sure," said Manager the Third. "But none of that was on our roadmap.""Well, maybe we should set up a meeting to go over the roadmap," Gary said. "Because a lot of the tasks on there don't make much sense right now-""I don't think that's a good use of time," Manager the Second said. "Large meetings are expensive. Just stick to the roadmap, please."With that, the meeting ended. Gary went back to work...... updating his resume. [Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.
CodeSOD: Off the Path
File path separators are a common pain point when writing cross platform software. Of course, not every programming language has a graceful API for handling that. For example, prior to C++ 17, you had to do some #ifdef preprocessor magic to handle that. Which people usually did (or they'd use the Boost suite of libraries).Code like this wouldn't be out of place or incorrect:
Representative Line: A Specific Key
Today's anonymous submission isn't really a WTF, but it highlights the hardest problem in computer science: naming things.For example, let's say you saw a method called handleRSAPrivateKeyGeneration. You'd likely assume that it generates an RSA private key. More specifically, it accepts a request for a private key and handles that request. It's right there in the name.
CodeSOD: The Most Dangerous Game
While we talk about bad video game code periodically, we generally avoid it because it's so specialized and while something like fast inverse square root is bad code from a maintainability perspective, it's great code for abusing floating points to make math fast.Itan Yldz sends us a snippet from a game's config file. I won't pick on the specific game, but this isn't some random build of TuxCart, but a released game sold on multiple platforms. It's from a small team, but it's an actual professional product running on many devices. What's notable about this is the game has multiplayer elements, which means networking code, which means...
Best of…: Classic WTF: Difficult Personality
CodeSOD: On Hold
"Dragoncoder" supports a web application that has a "wait time" for access. I hate that that's a thing, but I recognize that there are real-world constraints where this might make sense. Still, I hate it. But that's not the WTF.
CodeSOD: Module Test
TJ inherited a NestJS project. The original developers left the team many years ago, but they've left their mark in the codebase.
Flushed Out
While a project manager is frequently called upon for their planning ability, the real skill we want from project managers is their ability to communicate. The job of a project manager is to align the team doing the work, with the organization goals driving the work, with the management and leadership teams trying to understand the work, while juggling all the constraints like budgets, timelines, and the endlessly changing expectations for the project. A good project manager is worth their weight in gold. A bad one will cost their weight in gold.Mark was hired on as a contractor, reporting to Tegan. Tegan was fresh out of business school, complete with an MBA and a variety of project-management training certifications. Unfortunately for Mark and the rest of the team, and especially unfortunately for Tegan, she had absolutely no real world experience. To make matters worse, this wasn't just a software project: they were working on a system which matched newly developed software with newly designed mechanics and custom build control electronics. A group of experienced software engineers, mechanical engineers, and electrical engineers all found themselves reporting to a bright and shiny MBA. It's a role that she probably could have grown into, but management saw all the acronyms she continuously put after her name, and decided she could just take the whole thing over with no real guidance.It went badly pretty much from the beginning. Tegan was not a talented communicator. For example, Mark's team needed to know: on what timeline were the electrical engineers going to deliver the first prototypes, so the software team could start running bench tests of their software? Tegan's response was a fortune cookie message about balancing the complicated pipelines and lanes on the Gantt chart and hitting all of their milestones; like a fortune cookie, it was vague, important sounding, but ultimately empty.Of course, the natural reaction amongst the engineers was to just route around the damage: the various teams could talk to each other just fine without going through Tegan. That, unfortunately, did not go over well with management. Tegan, as the project manager, was their insight into the project. They needed her in the loop on everything. And she couldn't just be informed, she had an MBA. She needed to be making decisions. But she was unqualified to make those decisions, which meant the project gradually ground to a halt. Tegan's emails got more vague, her meetings got longer but accomplished less, and after a certain point, she just stopped replying to key email threads.The first few days of radio silence seemed like a gift. But as time passed and Tegan seemed uninterested or unable to reply to any of the questions the team had for her, the project started to flounder. The engineering teams escalated this problem to management. Management presumably went back to Tegan. At some point, feeling the weight of everything going wrong around her, Tegan sent out this email, which is definitely the best and clearest communication she managed during the project. It's arguably the clearest, and most accurate communication one could make in this situation:
CodeSOD: AAYFN
Jason M sends us some Ruby code.
CodeSOD: The Error Check
Today's submission is less a WTF and more a, "Yeah, that'd annoy me too."Stevie works in a code-base that's largely C, which means function return values are usually used to communicate to status codes. The standard:
CodeSOD: Wait Longer
Karen was maintaining some specification tests that were flaky. Not extremely flaky, but three or four times out of a thousand, the tests would just fail. The tests were complicated, and some of the operations were timing sensitive, so it wasn't precisely surprising- but the problem was that they were actually generous with their timing windows. The unit tests passed consistently, it was only these functional, specification-based tests that failed.So, for example, there were sections in the tests where they wanted to wait at least 2ms. Since the code and tests were in TypeScript, they used the setTimeout function, which per standard JavaScript documentation warns that it may wait longer. But again, Karen was fine with longer.Unfortunately for Karen, the documentation for NodeJS is less specific, as it makes no guarantees about when the timeout function gets invoked. This means that it can fire the timeout before the time has elapsed.After many, many hours of debugging, that was exactly the situation that Karen found herself in. Which is why her very simple wait function went from:
Classic WTF: My Many Girlfriends
CodeSOD: Convert Back, Way Back
Windows Presentation Foundation, the XML-based UI framework for Windows, has its own "fun" quirks. One of its core ideas is that controls can be data-bound: that text box is linked to a numeric field in your model class. Type a different number, and the model automagically updates.That's fine for what it is, but of course you're going to need to give it some instructions on how to do those kinds of conversions for your own custom types. And that's where the IValueConverter interface comes in.You can write a class which implements that interface, which can then Convert and ConvertBack. Which, as a note, I hate that naming convention; which way is "back"? Well, that's controlled via an annotation. This is some of Microsoft's sample code, from their docs:
Representative Line: Something Wonderful
Today, we look at a "representative comment" from Mark W. This particular comment appears on a function:
CodeSOD: Negative Days
Killian Brendel was looking through the .NET source code, and found this comment on the TimeSpan struct.
12345678