The Daily WTF
Coded Smorgasbord: Basically, a Smorgasbord
It's that time to take a look at a few short snippets.
Boolean values can hold true or false. But is that truly self documenting? I think we need clearer variable names for this. Certainly, the snippet Nonymous found thinks so:
boolean isTrue = false;Well, at least I'll know if it's true or not. I'm not sure what "it" is in this scenario, but I'm sure that's the least important part of all of this.
If you've worked in C#, you're aware that it offers both a string type, and a String type- they're the same thing. So Colin's co-worker isn't wrong for writing code this way, but they're also wrong for writing code this way.
writer.WriteLine(string.Empty); writer.WriteLine(String.Empty);Billie sends us this short bit of Java, which ensures that nulls are properly handled:
if (val == null) { return null; } return val;It's very important that, if val is null, we don't just return the contents of val, we should return null instead. Y'know, so no one is surprised by an unexpected null. Wait a second…
Finally, Jon finds this comment in the codebase. The code is elided, but I Jon has helpfully summarized it.
// Basically, … several thousand lines of dense code containing no further commentsHonestly, I'm not sure if that comment is a statement of surrender or just an ironic joke. Either way, I get it.
.comment { border: none; } [Advertisement] Plan Your .NET 9 Migration with ConfidenceYour journey to .NET 9 is more than just one decision.Avoid migration migraines with the advice in this free guide. Download Free Guide Now!
CodeSOD: Adding to the Argument
David G saw a pull request with a surprisingly long thread of comments on it. What was weirder was that the associated ticket was all about adding a single parameter to an existing method. What could be generating that much discussion?
How could adding an argument add to an argument?
registerFooWrapper: function(arg1, arg2, arg3, arg4, arg5, arg6, arg7) { bar.when('bar-event', function(context) { context.foo({ arg1: arg1, arg2: arg2, arg3: arg3, arg4: arg4, arg5: arg5, arg6: arg6, arg7: arg7, }); }); }This is the original version of the JavaScript function. The parameter names have been anonymized. That aside, this still isn't very good. Seven parameters is likely too many, and based on what I see in setting the context, there is an object type that holds them all, so maybe we should be passing the object around in the first place? Still, this isn't a WTF by any stretch, and since it's already deployed code, changing the interface significantly is a bad idea- maybe just adding a parameter is the right choice here. So what generated so much discussion?
This revision:
registerFooWrapper: function(arg1, arg2, arg3, arg4, arg5, arg6, arg7, notArg8) { if (notArg8 === true) { bar.when('bar-event', function(context) { context.foo({ arg1: arg1, arg2: arg2, arg3: arg3, arg4: arg4, arg5: arg5, arg6: arg6, arg7: arg7, arg8: !notArg8, }); }); } else { bar.when('bar-event', function(context) { context.foo({ arg1: arg1, arg2: arg2, arg3: arg3, arg4: arg4, arg5: arg5, arg6: arg6, arg7: arg7 }); }); } }Okay, so if notArg8 is true, we pass false to the context. If it's any other value, we don't past arg8 at all. I do not understand what I'm looking at here. If the goal is to ensure that arg8 is either true or not set, there are clearer ways to express that idea. But also, the goal of the ticket was not to do that- it was simply to add another parameter, which means you could drop the condition entirely and just add the parameter. context was already receiving arg8 as undefined, so it could clearly handle an undefined value.
David made some comments on the pull request, but the original developer just ended up going radio silent on it. One of the juniors on David's team approved it, for some reason, but nobody ever actually hit merge. Instead, a different developer simply made a version that took arg8 as a parameter, passed it down to context, and called it a day. It worked, the tests passed, and everyone was happy.
Well, except the original developer, but again, who knows what they were trying to do?
[Advertisement] ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.The Modern Job Hunt: Part 1
Ellis knew she needed a walk after she hurried off of Zoom at the end of the meeting to avoid sobbing in front of the group.
She'd just been attending a free online seminar regarding safe job hunting on the Internet. Having been searching since the end of January, Ellis had already picked up plenty of first-hand experience with the modern job market, one rejection at a time. She thought she'd attend the seminar just to see if there were any additional things she wasn't aware of. The seminar had gone well, good information presented in a clear and engaging way. But by the end of it, Ellis was feeling bleak. Goodness gracious, she'd already been slogging through months of this. Hundreds of job applications with nothing to show for it. All of the scams out there, all of the bad actors preying on people desperate for their and their loved ones' survival!
Ellis' childhood had been plagued with anxiety and depression. It was only as an adult that she'd learned any tricks for coping with them. These tricks had helped her avoid spiraling into full-on depression for the past several years. One such trick was to stop and notice whenever those first feelings hit. Recognize them, feel them, and then respond constructively.
First, a walk. Going out where there were trees and sunshine: Ellis considered this "garbage collection" for her brain. So she stepped out the front door and started down a tree-lined path near her house, holding on to that bleak feeling. She was well aware that if she didn't address it, it would take root and grow into hopelessness, self-loathing, fear of the future. It would paralyze her, leave her curled up on the couch doing nothing. And it would all happen without any words issuing from her inner voice. That was the most insidious thing. It happened way down deep in a place where there were no words at all.
Once she returned home, Ellis forced herself to sit down with a notebook and pencil and think very hard about what was bothering her. She wrote down each sentiment:
- This job search is a hopeless, unending slog!
- No one wants to hire me. There must be something wrong with me!
- This is the most brutal job search environment I've ever dealt with. There are new scams every day. Then add AI to every aspect until I want to vomit.
This was the first step of a reframing technique she'd just read about in the book Right Kind of Wrong by Amy Edmonson. With the words out, it was possible to look at each statement and determine whether it was rational or irrational, constructive or harmful. Each statement could be replaced with something better.
Ellis proceeded step by step through the list.
- Yes, this will end. Everything ends.
- There's nothing wrong with me. Most businesses are swamped with applications. There's a good chance mine aren't even being looked at before they're being auto-rejected. Remember the growth mindset you learned from Carol Dweck. Each application and interview is giving me experience and making me a better candidate.
- This job market is a novel context that changes every day. That means failure is not only inevitable, it's the only way forward.
Ellis realized that her job hunt was very much like a search algorithm trying to find a path through a maze. When the algorithm encountered a dead end, did it deserve blame? Was it an occasion for shame, embarrassment, and despair? Of course not. Simply backtrack and keep going with the knowledge gained.
Yes, there was truth to the fact that this was the toughest job market Ellis had ever experienced. Therefore, taking a note from Viktor Frankl, she spent a moment reimagining the struggle in a way that made it meaningful to her. Ellis began viewing her job hunt in this dangerous market, her gradual accumulation of survival information, as an act of resistance against it. She now hoped to write all about her experience once she was on the other side, in case her advice might help even one other person in her situation save time and frustration.
While unemployed, she also had the opportunity to employ the search algorithm against entirely new mazes. Could Ellis expand her freelance writing into a sustainable gig, for instance? That would mean exploring all the different ways to be a freelance writer, something Ellis was now curious and excited to explore.
[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.Best of…: Classic WTF: We Are Not Meatbots!
Sales, as everyone knows, is the mortal enemy of Development.
Their goals are opposite, their people are opposite, their tactics are opposite. Even their credos - developers "Make a good product" but sales will "Do anything to get that money" - are at complete odds.
The company Jordan worked for made a pseudo-enterprise product responsible for everything e-commerce: contacts, inventory, website, shipping, payment...everything. His responsibility included the inventory package, overseeing the development team, designing APIs, integration testing, and coordination with the DBAs and sysadmins...you know, everything. One of his team members implemented a website CMS into the product, letting the website design team ignore the content and focus on making it look good.
Care to guess who was responsible for the site content? If you guessed the VP of Sales, congratulations! You win a noprize.
A couple months passed by without incident. Everything's peachy in fact...that is, until one fateful day when the forty-person stock-and-shipping department are clustered in the parking lot when Jordan shows up.
Jordan parked, crossed the asphalt, and asked one of the less threatening looking warehouse guys, "What's the problem?"
The reply was swift as the entire group unanimously shouted "YOUR F***ING WEBSITE!" Another worker added, "You guys in EYE TEE are so far removed from real life out here. We do REAL WORK, what you guys do from behind your desks?"
Jordan was dumbfounded. What brought this on? For a moment he considered defending his and his team's honor but decided it wouldn't accomplish much besides get his face rearranged and instead replied with a meek "Sure, just let me check into this..." before quickly diving into the nearest entry door.
It didn't take much long after for Jordan to ascertain that the issue wasn't that the website was down, but that the content of one page in particular , the "About Us" page, had upset the hardworking staff who accomplished what the company actually promised: stock and ship the products that they sold on their clients' websites.
After an hour of mediation, it was discovered that the VP of Sales, in a strikingly-insensitive-even-for-him moment, had referred to the warehouse staff as "meatbots." The lively folk who staffed the shipping and stocking departments naturally felt disrespected by being reduced to some stupid sci-fi cloning trope nomenclature. The VP's excuse was simply that he had drunk a couple of beers while he wrote the page text for the website. Oops!
Remarkably, the company (which Jordan left some time later for unrelated reasons) eventually caught up to the backlog of orders to go out. It took a complete warehouse staff replacement, but they did catch up. Naturally, the VP of Sales is still there, with an even more impressive title.
photo credit: RTD Photography via photopin cc [Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!
Error'd: Scamproof
Gordon S. is smarter than the machines. "I can only presume the "Fix Now with AI" button adds some mistakes in order to fix the lack of needed fixes."
"Sorry, repost with the link https://www.daybreaker.com/alive/," wrote Michael R.
And yet again from Michael R., following up with a package mistracker. "Poor DHL driver. I hope he will get a break within those 2 days. And why does the van look like he's driving away from me."
Morgan
airs some dirty laundry.
"After navigating this washing machine app on holiday and validating
my credit card against another app I am greeted by this less than
helpful message each time. So is OK okay? Or is the Error in error?
Washing machine worked though."
And finally, scamproof Stuart wondered "Maybe the filter saw the word "scam" and immediately filed it into the scam bucket. All scams include the word "scam" in them, right?"
[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: Springs are Optional
Optional types are an attempt to patch the "billion dollar mistake". When you don't know if you have a value or not, you wrap it in an Optional, which ensures that there is a value (the Optional itself), thus avoiding null reference exceptions. Then you can query the Optional to see if there is a real value or not.
This is all fine and good, and can cut down on some bugs. Good implementations are loaded with convenience methods which make it easy to work on the optionals.
But then, you get code like Burgers found. Which just leaves us scratching our heads:
private static final Optional<Boolean> TRUE = Optional.of(Boolean.TRUE); private static final Optional<Boolean> FALSE = Optional.of(Boolean.FALSE);Look, any time you're making constants for TRUE or FALSE, something has gone wrong, and yes, I'm including pre-1999 versions of C in this. It's especially telling when you do it in a language that already has such constants, though- at its core- these lines are saying TRUE = TRUE. Yes, we're wrapping the whole thing in an Optional here, which potentially is useful, but if it is useful, something else has gone wrong.
Burgers works for a large insurance company, and writes this about the code:
I was trying to track down a certain piece of code in a Spring web API application when I noticed something curious. It looked like there was a chunk of code implementing an application-specific request filter in business logic, totally ignoring the filter functions offered by the framework itself and while it was not related to the task I was working on, I followed the filter apply call to its declaration. While I cannot supply the entire custom request filter implementation, take these two static declarations as a demonstration of how awful the rest of the class is.
Ah, of course- deep down, someone saw a perfectly functional wheel and said, "I could make one of those myself!" and these lines are representative of the result.
[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.CodeSOD: The HTML Print Value
Matt was handed a pile of VB .Net code, and told, "This is yours now. I'm sorry."
As often happens, previous company leadership said, "Why should I pay top dollar for experienced software engineers when I can hire three kids out of college for the same price?" The experiment ended poorly, and the result was a pile of bad VB code, which Matt now owned.
Here's a little taste:
// SET IN SESSION AND REDIRECT TO PRINT PAGE Session["PrintValue"] = GenerateHTMLOfItem(); Response.Redirect("PrintItem.aspx", true);The function name here is accurate. GenerateHTMLOfItem takes an item ID, generates the HTML output we want to use to render the item, and stores it in a session variable. It then forces the browser to redirect to a different page, where that HTML can then be output.
You may note, of course, that GenerateHTMLOfItem doesn't actually take parameters. That's because the item ID got stored in the session variable elsewhere.
Of course, it's the redirect that gets all the attention here. This is a client side redirect, so we generate all the HTML, shove it into a session object, and then send a message to the web browser: "Go look over here". The browser sends a fresh HTTP request for the new page, at which point we render it for them.
The Microsoft documentation also has this to add about the use of Response.Redirect(String, Boolean), as well:
Calling Redirect(String) is equivalent to calling Redirect(String, Boolean) with the second parameter set to true. Redirect calls End which throws a ThreadAbortException exception upon completion. This exception has a detrimental effect on Web application performance. Therefore, we recommend that instead of this overload you use the HttpResponse.Redirect(String, Boolean) overload and pass false for the endResponse parameter, and then call the CompleteRequest method. For more information, see the End method.
I love it when I see the developers do a bonus wrong.
Matt had enough fires to put out that fixing this particular disaster wasn't highest on his priority list. For the time being, he could only add this comment:
// SET IN SESSION AND REDIRECT TO PRINT PAGE // FOR THE LOVE OF GOD, WHY?!? Session["PrintValue"] = GenerateHTMLOfItem(); Response.Redirect("PrintItem.aspx", true); [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: Not What They Meant By Watching "AndOr"
Today's awfulness comes from Tim H, and while it's technically more than one line, it's so representative of the code, and so short that I'm going to call this a representative line. Before we get to the code, we need to talk a little history.
Tim's project is roughly three decades old. It's a C++ tool used for a variety of research projects, and this means that 90% of the people who have worked on it are PhD candidates in computer science programs. We all know the rule of CompSci PhDs and programming: they're terrible at it. It's like the old joke about the farmer who, when unable to find an engineer to build him a cow conveyer, asked a physicist. After months of work, the physicist introduced the result: "First, we assume a perfectly spherical cow in a vacuum…"
Now, this particularly function has been anonymized, but it's easy to understand what the intent was:
bool isFooOrBar() { return isFoo() && isBar(); }The obvious problem here is the mismatch between the function name and the actual function behavior- it promises an or operation, but does an and, which the astute reader may note are different things.
I think this offers another problem, though. Even if the function name were correct, given the brevity of the body, I'd argue that it actually makes the code less clear. Maybe it's just me, but isFoo() && isBar() is more clear in its intent than isFooAndBar(). There's a cognitive overhead to adding more symbols that would make me reluctant to add such a function.
There may be an argument about code-reuse, but it's worth noting- this function is only ever called in one place.
This particular function is not itself, all that new. Tim writes:
This was committed as new code in 2010 (i.e., not a refactor). I'm not sure if the author changed their mind in the middle of writing the function or just forgot which buttons on the keyboard to press.
More likely, Tim, is that they initially wrote it as an "or" operation and then discovered that they were wrong and it needed to be an "and". Despite the fact that the function was only called in one place, they opted to change the body without changing the name, because they didn't want to "track down all the places it's used". Besides, isn't the point of a function to encapsulate the behavior?
[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!The C-Level Ticket
Everyone's got workplace woes. The clueless manager; the disruptive coworker; the cube walls that loom ever higher as the years pass, trapping whatever's left of your soul.
But sometimes, Satan really leaves his mark on a joint. I worked Tech Support there. This is my story. Who am I? Just call me Anonymous.
It starts at the top. A call came in from Lawrence Gibbs, the CEO himself, telling us that a conference room printer was, quote, "leaking." He didn't explain it, he just hung up. The boss ordered me out immediately, told me to step on it. I ignored the elevator, racing up the staircase floor after floor until I reached the dizzying summit of C-Town.
There's less oxygen up there, I'm sure of it. My lungs ached and my head spun as I struggled to catch my breath. The fancy tile and high ceilings made a workaday schmuck like me feel daunted, unwelcome. All the same, I gathered myself and pushed on, if only to learn what on earth "leaking" meant in relation to a printer.
I followed the signs on the wall to the specified conference room. In there, the thermostat had been kicked down into the negatives. The cold cut through every layer of mandated business attire, straight to bone. The scene was thick with milling bystanders who hugged themselves and traded the occasional nervous glance. Gibbs was nowhere to be found.
Remembering my duty, I summoned my nerve. "Tech Support. Where's the printer?" I asked.
Several pointing fingers showed me the way. The large printer/scanner was situated against the far wall, flanking an even more enormous conference table. Upon rounding the table, I was greeted with a grim sight: dozens of sheets of paper strewn about the floor like blood spatter. Everyone was keeping their distance; no one paid me any mind as I knelt to gather the pages. There were 30 in all. Each one was blank on one side, and sported some kind of large, blotchy ring on the other. Lord knew I drank enough java to recognize a coffee mug stain when I saw one, but these weren't actual stains. They were printouts of stains.
The printer was plugged in. No sign of foul play. As I knelt there, unseen and unheeded, I clutched the ruined papers to my chest. Someone had wasted a tree and a good bit of toner, and for what? How'd it go down? Surely Gibbs knew more than he'd let on. The thought of seeking him out, demanding answers, set my heart to pounding. It was no good, I knew. He'd play coy all day and hand me my pink slip if I pushed too hard. As much as I wanted the truth, I had a stack of unpaid bills at home almost as thick as the one in my arms. I had to come up with something else.
There had to be witnesses among the bystanders. I stood up and glanced among them, seeking out any who would return eye contact. There: a woman who looked every bit as polished as everyone else. But for once, I got the feeling that what lay beneath the facade wasn't rotten.
With my eyes, I pleaded for answers.
Not here, her gaze pleaded back.
I was getting somewhere, I just had to arrange for some privacy. I hurried around the table again and weaved through bystanders toward the exit, hoping to beat it out of that icebox unnoticed. When I reached the threshold, I spotted Gibbs charging up the corridor, smoldering with entitlement. "Where the hell is Tech Support?!"
I froze a good distance away from the oncoming executive, whose voice I recognized from a thousand corporate presentations. Instead of putting me to sleep this time, it jolted down my spine like lightning. I had to think fast, or I was gonna lose my lead, if not my life.
"I'm right here, sir!" I said. "Be right back! I, uh, just need to find a folder for these papers."
"I've got one in my office."
A woman's voice issued calmly only a few feet behind me. I spun around, and it was her, all right, her demeanor as cool as our surroundings. She nodded my way. "Follow me."
My spirits soared. At that moment, I would've followed her into hell. Turning around, I had the pleasure of seeing Gibbs stop short with a glare of contempt. Then he waved us out of his sight.
Once we were out in the corridor, she took the lead, guiding me through the halls as I marveled at my luck. Eventually, she used her key card on one of the massive oak doors, and in we went.
You could've fit my entire apartment into that office. The place was spotless. Mini-fridge, espresso machine, even couches: none of it looked used. There were a couple of cardboard boxes piled up near her desk, which sat in front of a massive floor-to-ceiling window admitting ample sunlight.
She motioned toward one of the couches, inviting me to sit. I shook my head in reply. I was dying for a cigarette by that point, but I didn't dare light up within this sanctuary. Not sure what to expect next, I played it cautious, hovering close to the exit. "Thanks for the help back there, ma'am."
"Don't mention it." She walked back to her desk, opened up a drawer, and pulled out a brand-new manila folder. Then she returned to conversational distance and proffered it my way. "You're from Tech Support?"
There was pure curiosity in her voice, no disparagement, which was encouraging. I accepted the folder and stuffed the ruined pages inside. "That's right, ma'am."
She shook her head. "Please call me Leila. I started a few weeks ago. I'm the new head of HR."
That acronym, which usually put me on edge, somehow failed to raise my hackles. I'd have to keep vigilant, of course, but so far she seemed surprisingly OK. "Welcome aboard, Leila. I wish we were meeting in better circumstances." Duty beckoned. I hefted the folder. "Printers don't just leak."
"No." Leila glanced askance, grave.
"Tell me what you saw."
"Well ..." She shrugged helplessly. "Whenever Mr. Gibbs gets excited during a meeting, he tends to lean against the printer and rest his coffee mug on top of it. Today, he must've hit the Scan button with his elbow. I saw the scanner go off. It was so bright ..." She trailed off with a pained glance downward.
"I know this is hard," I told her when the silence stretched too long. "Please, continue."
Leila summoned her mettle. "After he leaned on the controls, those pages spilled out of the printer. And then ... then somehow, I have no idea, I swear! Somehow, all those pages were also emailed to me, Mr. Gibbs' assistant, and the entire board of directors!"
The shock hit me first. My eyes went wide and my jaw fell. But then I reminded myself, I'd seen just as crazy and worse as the result of a cat jumping on a keyboard. A feline doesn't know any better. A top-level executive, on the other hand, should know better.
"Sounds to me like the printer's just fine," I spoke with conviction. "What we have here is a CEO who thinks it's OK to treat an expensive piece of office equipment like his own personal fainting couch."
"It's terrible!" Leila's gaze burned with purpose. "I promise, I'll do everything I possibly can to make sure something like this never happens again!"
I smiled a gallows smile. "Not sure what anyone can do to fix this joint, but the offer's appreciated. Thanks again for your help."
Now that I'd seen this glimpse of better things, I selfishly wanted to linger. But it was high time I got outta there. I didn't wanna make her late for some meeting or waste her time. I backed up toward the door on feet that were reluctant to move.
Leila watched me with a look of concern. "Mr. Gibbs was the one who called Tech Support. I can't close your ticket for you; you'll have to get him to do it. What are you going to do?"
She cared. That made leaving even harder. "I dunno yet. I'll think of something."
I turned around, opened the massive door, and put myself on the other side of it in a hurry, using wall signs to backtrack to the conference room. Would our paths ever cross again? Unlikely. Someone like her was sure to get fired, or quit out of frustration, or get corrupted over time.
It was too painful to think about, so I forced myself to focus on the folder of wasted pages in my arms instead. It felt like a mile-long rap sheet. I was dealing with an alleged leader who went so far as to blame the material world around him rather than accept personal responsibility. I'd have to appeal to one or more of the things he actually cared about: himself, his bottom line, his sense of power.
By the time I returned to the conference room to face the CEO, I knew what to tell him. "You're right, sir, there's something very wrong with this printer. We're gonna take it out here and give it a thorough work-up."
That was how I was able to get the printer out of that conference room for good. Once it underwent "inspection" and "testing," it received a new home in a previously unused closet. Whenever Gibbs got to jawing in future meetings, all he could do was lean against the wall. Ticket closed.
Gibbs remained at the top, doing accursed things that trickled down to the roots of his accursed company. But at least from then on, every onboarding slideshow included a photo of one of the coffee ring printouts, with the title Respect the Equipment.
Thanks, Leila. I can live with that.
[Advertisement] Picking up NuGet is easy. Getting good at it takes time. Download our guide to learn the best practice of NuGet for the Enterprise.Error'd: 8 Days a Week
"What word can spell with the letters housucops?" asks Mark R. "Sometimes AI hallucinations can be hard to find. Other times, they just kind of stand out..."
"Do I need more disks?" wonders Gordon "I'm replacing a machine which has only 2 GB of HDD. New one has 2 TB, but that may not be enough. Unless Thunar is lying." It's being replaced by an LLM.
"Greenmobility UX is a nightmare" complains an anonymous reader. "Just like last week's submission, do you want to cancel? Cancel or Leave?" This is not quite as bad as last week's.
Cinephile jeffphi rated this film two thumbs down. "This was a very boring preview, cannot recommend."
Malingering Manuel H. muses "Who doesn't like long weekends? Sometimes, one Sunday per week is just not enough, so just put a second one right after the first." I don't want to wait until Oktober for a second Sunday; hope we get one søøn.
[Advertisement] ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.
A Countable
Once upon a time, when the Web was young, if you wanted to be a cool kid, you absolutely needed two things on your website: a guestbook for people to sign, and a hit counter showing how many people had visited your Geocities page hosting your Star Trek fan fiction.
These days, we don't see them as often, but companies still like to track the information, especially when it comes to counting downloads. So when Justin started on a new team and saw a download count in their analytics, he didn't think much of it at all. Nor did he think much about it when he saw the download count displayed on the download page.
Another thing that Justin didn't think much about was big piles of commits getting merged in overnight, at least not at first. But each morning, Justin needed to pull in a long litany of changes from a user named "MrStinky". For the first few weeks, Justin was too preoccupied with getting his feet under him, so he didn't think about it too much.
But eventually, he couldn't ignore what he saw in the git logs.
docs: update download count to 51741 docs: update download count to 51740 docs: update download count to 51738And each commit was exactly what the name implied, a diff like:
- 51740 + 51741Each time a user clicked the download link, a ping was sent to their analytics system. Throughout the day, the bot "MrStinky" would query the analytics tool, and create new commits that updated the counter. Overnight, it would bundle those commits into a merge request, approve the request, merge the changes, and then redeploy what was at the tip of main.
"But, WHY?" Justin asked his peers.
One of them just shrugged. "It seemed like the easiest and fastest way at the time?"
"I wanted to wire Mr Stinky up to our content management system's database, but just never got around to it. And this works fine," said another.
Much like the rest of the team, Justin found that there were bigger issues to tackle.
[Advertisement] Plan Your .NET 9 Migration with ConfidenceYour journey to .NET 9 is more than just one decision.Avoid migration migraines with the advice in this free guide. Download Free Guide Now!
CodeSOD: Copy of a Copy of a
Jessica recently started at a company still using Windows Forms.
Well, that was a short article. Oh, you want more WTF than that? Sure, we can do that.
As you might imagine, a company that's still using Windows Forms isn't going to upgrade any time soon; they've been using an API that's been in maintenance mode for a decade, clearly they're happy with it.
But they're not too happy- Jessica was asked to track down a badly performing report. This of course meant wading through a thicket of spaghetti code, pointless singletons, and the general sloppiness that is the code base. Some of the code was written using Entity Framework for database access, much of it is not.
While it wasn't the report that Jessica was sent to debug, this method caught her eye:
private Dictionary<long, decimal> GetReportDiscounts(ReportCriteria criteria) { Dictionary<long, decimal> rows = new Dictionary<long, decimal>(); string query = @"select ii.IID, SUM(CASE WHEN ii.AdjustedTotal IS NULL THEN (ii.UnitPrice * ii.Units) ELSE ii.AdjustedTotal END) as 'Costs' from ii where ItemType = 3 group by ii.IID "; string connectionString = string.Empty; using (DataContext db = DataContextFactory.GetInstance<DataContext>()) { connectionString = db.Database.Connection.ConnectionString; } using (SqlConnection connection = new SqlConnection(connectionString)) { using (SqlCommand command = new SqlCommand(query, connection)) { command.Parameters.AddWithValue("@DateStart", criteria.Period.Value.Min.Value.Date); command.Parameters.AddWithValue("@DateEnd", criteria.Period.Value.Max.Value.Date.AddDays(1)); command.Connection.Open(); using (SqlDataReader reader = command.ExecuteReader()) { while (reader.Read()) { decimal discount = (decimal)reader["Costs"]; long IID = (long)reader["IID"]; if (rows.ContainsKey(IID)) { rows[IID] += discount; } else { rows.Add(IID, discount); } } } } } return rows; }This code constructs a query, opens a connection, runs the query, and iterates across the results, building a dictionary as its result set. The first thing which leaps out is that, in code, they're doing a summary (iterating across the results and grouping by IID), which is also what they did in the query.
It's also notable that the table they're querying is called ii, which is not a result of anonymization, and actually what they called it. Then there's the fact that they set parameters on the query, for DateStart and DateEnd, but the query doesn't use those. And then there's that magic number 3 in the query, which is its own set of questions.
Then, right beneath that method was one called GetReportTotals. I won't share it, because it's identical to what's above, with one difference:
string query = @" select ii.IID, SUM(CASE WHEN ii.AdjustedTotal IS NULL THEN (ii.UnitPrice * ii.Units) ELSE ii.AdjustedTotal END) as 'Costs' from ii where itemtype = 0 group by iid ";The magic number is now zero.
So, clearly we're in the world of copy/paste programming, but this raises the question: which came first, the 0 or the 3? The answer is neither. GetCancelledInvoices came first.
private List<ReportDataRow> GetCancelledInvoices(ReportCriteria criteria, Dictionary<long, string> dictOfInfo) { List<ReportDataRow> rows = new List<ReportDataRow>(); string fCriteriaName = "All"; string query = @"select A long query that could easily be done in EF, or at worst a stored procedure or view. Does actually use the associated parameters"; string connectionString = string.Empty; using (DataContext db = DataContextFactory.GetInstance<DataContext>()) { connectionString = db.Database.Connection.ConnectionString; } using (SqlConnection connection = new SqlConnection(connectionString)) { using (SqlCommand command = new SqlCommand(query, connection)) { command.Parameters.AddWithValue("@DateStart", criteria.Period.Value.Min.Value.Date); command.Parameters.AddWithValue("@DateEnd", criteria.Period.Value.Max.Value.Date.AddDays(1)); command.Connection.Open(); using (SqlDataReader reader = command.ExecuteReader()) { while (reader.Read()) { long ID = (long)reader["ID"]; decimal costs = (decimal)reader["Costs"]; string mNumber = (string)reader["MNumber"]; string mName = (string)reader["MName"]; DateTime idate = (DateTime)reader["IDate"]; DateTime lastUpdatedOn = (DateTime)reader["LastUpdatedOn"]; string iNumber = reader["INumber"] is DBNull ? string.Empty : (string)reader["INumber"]; long fId = (long)reader["FID"]; string empName = (string)reader["EmpName"]; string empNumber = reader["EmpNumber"] is DBNull ? string.Empty : (string)reader["empNumber"]; long mId = (long)reader["MID"]; string cName = dictOfInfo[matterId]; if (criteria.EmployeeID.HasValue && fId != criteria.EmployeeID.Value) { continue; } rows.Add(new ReportDataRow() { CName = cName, IID = ID, Costs = costs * -1, //Cancelled i - minus PC TimedValue = 0, MNumber = mNumber, MName = mName, BillDate = lastUpdatedOn, BillNumber = iNumber + "A", FID = fId, EmployeeName = empName, EmployeeNumber = empNumber }); } } } } return rows; }This is the original version of the method. We can infer this because it actually uses the parameters of DateStart and DateEnd. Everything else just copy/pasted this method and stripped out bits until it worked. There are more children of this method, each an ugly baby of its own, but all alike in their ugliness.
It's also worth noting, the original version is doing filtering after getting data from the database, instead of putting that criteria in the WHERE clause.
As for Jessica's poor performing report, it wasn't one of these methods. It was, however, another variation on "run a query, then filter, sort, and summarize in C#". By simply rewriting it as a SQL query in a stored procedure that leveraged indexes, performance improved significantly.
[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.CodeSOD: I Am Not 200
In theory, HTTP status codes should be easy to work with. In the 100s? You're doing some weird stuff and breaking up large requests into multiple sub-requests. 200s? It's all good. 300s? Look over there. 400s? What the hell are you trying to do? 500s? What the hell is the server trying to do?
This doesn't mean people don't endlessly find ways to make it hard. LinkedIn, for example, apparently likes to send 999s if you try and view a page without being logged in. Shopify has invented a few. Apache has added a 218 "This is Fine". And then there's WebDAV, which not only adds new status codes, but adds a whole bunch of new verbs to HTTP requests.
Francesco D sends us a "clever" attempt at handling status codes.
try { HttpRequest.Builder localVarRequestBuilder = {{operationId}}RequestBuilder({{#allParams}}{{paramName}}{{^-last}}, {{/-last}}{{/allParams}}{{#hasParams}}, {{/hasParams}}headers); return memberVarHttpClient.sendAsync( localVarRequestBuilder.build(), HttpResponse.BodyHandlers.ofString()).thenComposeAsync(localVarResponse -> { if (localVarResponse.statusCode()/ 100 != 2) { return CompletableFuture.failedFuture(getApiException("{{operationId}}", localVarResponse)); } {{#returnType}} try { String responseBody = localVarResponse.body(); return CompletableFuture.completedFuture( responseBody == null || responseBody.isBlank() ? null : memberVarObjectMapper.readValue(responseBody, new TypeReference<{{{returnType}}}>() {}) ); } catch (IOException e) { return CompletableFuture.failedFuture(new ApiException(e)); } {{/returnType}} {{^returnType}} return CompletableFuture.completedFuture(null); {{/returnType}} }); }Okay, before we get to the status code nonsense, I first have to whine about this templating language. I'm generally of the mind that generated code is a sign of bad abstractions, especially if we're talking about using a text templating engine, like this. I'm fine with hygienic macros, and even C++'s templating system for code generation, because they exist within the language. But fine, that's just my "ok boomer" opinion, so let's get into the real meat of it, which is this line:
localVarResponse.statusCode()/ 100 != 2"Hey," some developer said, "since success is in the 200 range, I'll just divide by 100, and check if it's a 2, helpfully truncating the details." Which is fine and good, except neither 100s nor 300s represent a true error, especially because if the local client is doing caching, a 304 tells us that we can used the cached version.
For Francesco, treating 300s as an error created a slew of failed requests which shouldn't have failed. It wasn't too difficult to detect- they were at least logging the entire response- but it was frustrating, if only because it seems like someone was more interested in being clever with math than actually writing good software.
[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.CodeSOD: Going Crazy
For months, everything at Yusuf's company was fine. Then, suddenly, he comes in to the office to learn that overnight the log exploded with thousands of panic messages. No software changes had been pushed, no major configurations had happened- just a reboot. What had gone wrong?
This particular function was invoked as part of the application startup:
func (a *App) setupDocDBClient(ctx context.Context) error { docdbClient, err := docdb.NewClient( ctx, a.config.MongoConfig.URI, a.config.MongoConfig.Database, a.config.MongoConfig.EnableTLS, ) if err != nil { return nil } a.DocDBClient = docdbClient return nil }This is Go, which passes errors as part of the return. You can see an example where docdb.NewClient returns a client and an err object. At one point in the history of this function, it did the same thing- if connecting to the database failed, it returned an error.
But a few months earlier, an engineer changed it to swallow the error- if an error occurred, it would return nil.
As an organization, they did code reviews. Multiple people looked at this and signed off- or, more likely, multiple people clicked a button to say they'd looked at it, but hadn't.
Most of the time, there weren't any connection issues. But sometimes there were. One reboot had a flaky moment with connecting, and the error was ignored. Later on in execution, downstream modules started failing, which eventually lead to a log full of panic level messages.
The change was part of a commit tagged merely: "Refactoring". Something got factored, good and hard, all right.
[Advertisement] Picking up NuGet is easy. Getting good at it takes time. Download our guide to learn the best practice of NuGet for the Enterprise.Error'd: Abort, Cancel, Fail?
low-case jeffphi found "Yep, all kinds of technical errors."
Michael R. reports an off by 900 error.
"It is often said that news slows down in August," notes Stewart , wondering if "perhaps The Times have just given up? Or perhaps one of the biggest media companies just doesn't care about their paying subscribers?"
"Zero is a dangerous idea!" exclaims Ernie in Berkeley .
Daniel D. found one of my unfavorites, calling it "Another classic case of cancel dialog. This time featuring KDE Partition Manager."
Fail? Until next time. [Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!
CodeSOD: An Array of Parameters
Andreas found this in a rather large, rather ugly production code base.
private static void LogView(object o) { try { ArrayList al = (ArrayList)o; int pageId = (int)al[0]; int userId = (int)al[1]; // ... snipped: Executing a stored procedure that stores the values in the database } catch (Exception) { } }This function accepts an object of any type, except no, it doesn't, it expect that object to be an ArrayList. It then assumes the array list will then store values in a specific order. Note that they're not using a generic ArrayList here, nor could they- it (potentially) needs to hold a mix of types.
What they've done here is replace a parameter list with an ArrayList, giving up compile time type checking for surprising runtime exceptions. And why?
"Well," the culprit explained when Andreas asked about this, "the underlying database may change. And then the function would need to take different parameters. But that could break existing code, so this allows us to add parameters without ever having to change existing code."
"Have you heard of optional arguments?" Andreas asked.
"No, all of our arguments are required. We'll just default the ones that the caller doesn't supply."
And yes, this particular pattern shows up all through the code base. It's "more flexible this way."
.comment { border: none; } [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.CodeSOD: Raise VibeError
Ronan works with a vibe coder- an LLM addicted developer. This is a type of developer that's showing up with increasing frequency. Their common features include: not reading the code the AI generated, not testing the code the AI generated, not understanding the context of the code or how it integrates into the broader program, and absolutely not bothering to follow the company coding standards.
Here's an example of the kind of Python code they were "writing":
if isinstance(o, Test): if o.requirement is None: logger.error(f"Invalid 'requirement' in Test: {o.key}") try: raise ValueError("Missing requirement in Test object.") except ValueError: pass if o.title is None: logger.error(f"Invalid 'title' in Test: {o.key}") try: raise ValueError("Missing title in Test object.") except ValueError: passAn isinstance check, is already a red flag. Even without proper type annotations and type checking (though you should use them) any sort of sane coding is going to avoid situations where your method isn't sure what input it's getting. isinstance isn't a WTF, but it's a hint at something lurking off screen. (Yes, sometimes you do need it, this may be one of those times, but I doubt it.)
In this case, if the Test object is missing certain fields, we want to log errors about it. That part, honestly, is all fine. There are potentially better ways to express this idea, but the idea is fine.
No, the obvious turd in the punchbowl here is the exception handling. This is pure LLM, in that it's a statistically probable result of telling the LLM "raise an error if the requirement field is missing". The resulting code, however, raises an exception, immediately catches it, and then does nothing with it.
I'd almost think it's a pre-canned snippet that's meant to be filled in, but no- there's no reason a snippet would throw and catch the same error.
Now, in Ronan's case, this has a happy ending: after a few weeks of some pretty miserable collaboration, the new developer got fired. None of "their" code ever got merged in. But they've already got a few thousand AI generated resumes out to new positions…
[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.CodeSOD: Round Strips
JavaScript is frequently surprising in terms of what functions it does not support. For example, while it has a Math.round function, that only rounds to the nearest integer, not an arbitrary precision. That's no big deal, of course, as if you wanted to round to, say, four decimal places, you could write something like: Math.floor(n * 10000) / 10000.
But in the absence of a built-in function to handle that means that many developers choose to reinvent the wheel. Ryan found this one.
function stripExtraNumbers(num) { //check if the number's already okay //assume a whole number is valid var n2 = num.toString(); if(n2.indexOf(".") == -1) { return num; } //if it has numbers after the decimal point, //limit the number of digits after the decimal point to 4 //we use parseFloat if strings are passed into the method if(typeof num == "string"){ num = parseFloat(num).toFixed(4); } else { num = num.toFixed(4); } //strip any extra zeros return parseFloat(num.toString().replace(/0*$/,"")); }We start by turning the number into a string and checking for a decimal point. If it doesn't have one, we've already rounded off, return the input. Now, we don't trust our input, so if the input was already a string, we'll parse it into a number. Once we know it's a number, we can call toFixed, which returns a string rounded off to the correct number of decimal points.
This is all very dumb. Just dumb. But it's the last line which gets really dumb.
toFixed returns a padded string, e.g. (10).toFixed(4) returns "10.0000". But this function doesn't want those trailing zeros, so they convert our string num into a string, then use a regex to replace all of the trailing zeros, and then parse it back into a float.
Which, of course, when storing the number as a number, we don't really care about trailing zeros. That's a formatting choice when we output it.
I'm always impressed by a code sample where every single line is wrong. It's like a little treat. In this case, it even gets me a sense of how it evolved from little snippets of misunderstood code. The regex to remove trailing zeros in some other place in this developer's experience led to degenerate cases where they had output like 10., so they also knew they needed to have the check at the top to see if the input had a fractional part. Which the only way they knew to do that was by looking for a . in a string (have fun internationalizing that!). They also clearly don't have a good grasp on types, so it makes sense that they have the extra string check, just to be on the safe side (though it's worth noting that parseFloat is perfectly happy to run on a value that's already a float).
This all could be a one-liner, or maybe two if you really need to verify your types. Yet here we are, with a delightfully wrong way to do everything.
.comment { border: none; } [Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.CodeSOD: A Single Lint Problem
We've discussed singleton abuse as an antipattern many times on this site, but folks keep trying to find new ways to implement them badly. And Olivia's co-worker certainly found one.
We start with a C++ utility class with a bunch of functions in it:
//utilities.h class CUtilities { public CUtilities(); void doSomething(); void doSomeOtherThing(); }; extern CUtilities* g_Utility;So yes, if you're making a pile of utility methods, or if you want a singleton object, the keyword you're looking for is static. We'll set that aside. This class declares a class, and then also declares that there will be a pointer to the class, somewhere.
We don't have to look far.
//utilities.cpp CUtilities* g_Utility = nullptr; CUtilities::CUtilities() { g_Utility = this; } // all my do-whatever functions hereThis defines the global pointer variable, and then also writes the constructor of the utility class so that it initializes the global pointer to itself.
It's worth noting, at this point, that this is not a singleton, because this does nothing to prevent multiple instances from being created. What it does guarantee is that for each new instance, we overwrite g_Utility without disposing of what was already in there, which is a nice memory leak.
But where, or where, does the constructor get called?
//startup.h class CUtilityInit { private: CUtilities m_Instance; }; //startup.cpp CUtilityInit *utils = new CUtilityInit();I don't hate a program that starts with an initialization step that clearly instantiates all the key objects. There's just one little problem here that we'll come back to in just a moment, but let's look at the end result.
Anywhere that needs the utilities now can do this:
#include "utilities.h" //in the code g_Utility->doSomething();There's just one key problem: back in the startup.h, we have a private member called CUtilities m_Instance which is never referenced anywhere else in the code. This means when people, like Olivia, are trawling through the codebase looking for linter errors they can fix, they may see an "unused member" and decide to remove it. Which is what Olivia did.
The result compiles just fine, but explodes at runtime since g_Utility was never initialized.
The fix was simple: just don't try and make this a singleton, since it isn't one anyway. At startup, she just populated g_Utility with an instance, and threw away all the weird code around populating it through construction.
Singletons are, as a general rule, bad. Badly implemented singletons themselves easily turn into landmines waiting for unwary developers. Stop being clever and don't try and apply a design pattern for the sake of saying you used a design pattern.
.comment { border: none; } [Advertisement] Picking up NuGet is easy. Getting good at it takes time. Download our guide to learn the best practice of NuGet for the Enterprise.Error'd: Voluntold
It is said (allegedly by the Scots) that confession heals the soul. But does it need to be strictly voluntary? The folks toiling away over at CodeSOD conscientiously change the names to protect the innocent but this side of the house is committed to curing the tortured souls of webdevs. Whether they like it or not. Sadly Sam's submission has been blinded, so the black black soul of xxxxxxxxxxte.com remains unfortunately undesmirched, but three others should be experiencing the sweet sweet healing light of day right about now. I sure hope they appreciate it.
More monkey business this week from Reinier B. who is hoping to check in on some distant cousins. "I'll make sure to accept email from {email address}, otherwise I won't be able to visit {zoo name}."
Alex A. is "trying to pay customs duty." It definitely can be.
"I know it's hard to recruit good developers," commiserates Sam B. , "but it's like they're not even trying." They sure are, as above.
Peter G. bemoans "Apparently this network power thingamajig, found on Aliexpress, is pain itself if the brand name on it is to be believed." Cicero wept.
Jan B. takes the perfecta, hitting not only this week's theme of flubstitutions but also a bounty of bodged null references to boot. "This is one of the hardest choices I've ever had in my life. I'm not sure if I'd prefer null or null as my location.detection.message.CZ." Go in peace.
[Advertisement] ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.