Error'd: On the Dark Side
...matter of fact, it's all dark.
Gitter Hubber checks in on the holidays: "This is the spirit of the Black Friday on GitHub. That's because I'm using dark mode. Otherwise, it would have a different name… You know what? Let's just call it Error Friday!"
"Best get typing!" self-admonishes.
Jason G.
Suffering a surfeit of snark, he proposes
"Not sure my battery will last long enough.
Finally, quantum resistant security.
I can't remember my number after the 5000th digit.
" Any of those will do just fine.
Don't count Calle L. out. "This is for a calorie tracking app, on Thanksgiving. Offer was so delicious it wasn't even a number any more! Sadly it did not slim the price down more than expected."
"Snow and rain and rain and snow!" exclaims Paul N. "Weather so astounding, they just had to trigger three separate notifications at the same time."
It's not a holiday for everyone though, is it? Certainly not for Michael R. , who is back with a customer service complaint about custom deliveries. "I am unlucky with my deliveries. This time it's DPD. "
[Advertisement] Plan Your .NET 9 Migration with Confidence
Your journey to .NET 9 is more than just one decision.Avoid migration migraines with the advice in this free guide. Download Free Guide Now!
Classic WTF: Teleported Release
Matt works at an accounting firm, as a data engineer. He makes reports for people who don’t read said reports. Accounting firms specialize in different areas of accountancy, and Matt’s firm is a general firm with mid-size clients.
The CEO of the firm is a legacy from the last century. The most advanced technology on his desk is a business calculator and a pencil sharpener. He still doesn’t use a cellphone. But he does have a son, who is “tech savvy”, which gives the CEO a horrible idea of how things work.
Usually, this is pretty light, in that it’s sorting Excel files or sorting the output of an existing report. Sometimes the requests are bizarre or utter nonsense. And, because the boss doesn’t know what the technical folks are doing, some of the IT staff may be a bit lazy about following best practices.
This means that most of Matt’s morning is spent doing what is essentially Tier 1 support before he gets into doing his real job. Recently, there was a worse crunch, as actual support person Lucinda was out for materinity leave, and Jackie, the one other developer, was off on vacation on a foreign island with no Internet. Matt was in the middle of eating a delicious lunch of take-out lo mein when his phone rang. He sighed when he saw the number.
“Matt!” the CEO exclaimed. “Matt! We need to do a build of the flagship app! And a deploy!”
The app was rather large, and a build could take upwards of 45 minutes, depending on the day and how the IT gods were feeling. But the process was automated, the latest changes all got built and deployed each night. Anything approved was released within 24 hours. With everyone out of the office, there hadn’t been any approved changes for a few weeks.
Matt checked the Github to see if something went wrong with the automated build. Everything was fine.
“Okay, so I’m seeing that everything built on GitHub and everything is available in production,” Matt said.
“I want you to do a manual build, like you used to.”
“If I were to compile right now, it could take quite awhile, and redeploying runs the risk of taking our clients offline, and nothing would be any different.”
“Yes, but I want a build that has the changes which Jackie was working on before she left for vacation.”
Matt checked the commit history, and sure enough, Jackie hadn’t committed any changes since two weeks before leaving on vacation. “It doesn’t looked like she pushed those changes to Github.”
“Githoob? I thought everything was automated. You told me the process was automated,” the CEO said.
“It’s kind of like…” Matt paused to think of an analogy that could explain this to a golden retriever. “Your dishwasher, you could put a timer on it to run it every night, but if you don’t load the dishwasher first, nothing gets cleaned.”
There was a long pause as the CEO failed to understand this. “I want Jackie’s front-page changes to be in the demo I’m about to do. This is for Initech, and there’s millions of dollars riding on their account.”
“Well,” Matt said, “Jackie hasn’t pushed- hasn’t loaded her metaphorical dishes into the dishwasher, so I can’t really build them.”
“I don’t understand, it’s on her computer. I thought these computers were on the cloud. Why am I spending all this money on clouds?”
“If Jackie doesn’t put it on the cloud, it’s not there. It’s uh… like a fax machine, and she hasn’t sent us the fax.”
“Can’t you get it off her laptop?”
“I think she took it home with her,” Matt said.
“So?”
“Have you ever seen Star Trek? Unless Scotty can teleport us to Jackie’s laptop, we can’t get at her files.”
The CEO locked up on that metaphor. “Can’t you just hack into it? I thought the NSA could do that.”
“No-” Matt paused. Maybe Matt could try and recreate the changes quickly? “How long before this meeting?” he asked.
“Twenty minutes.”
“Just to be clear, you want me to do a local build with files I don’t have by hacking them from a computer which may or may not be on and connected to the Internet, and then complete a build process which usually takes 45 minutes- at least- deploy to production, so you can do a demo in twenty minutes?”
“Why is that so difficult?” the CEO demanded.
“I can call Jackie, and if she answers, maybe we can figure something out.”
The CEO sighed. “Fine.”
Matt called Jackie. She didn’t answer. Matt left a voicemail and then went back to eating his now-cold lo mein.
[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.Announcements: We Want Your Holiday Horrors
As we enter into the latter portion of the year, folks are traveling to visit family, logging off of work in hopes that everything can look after itself for a month, and somewhere, someone, is going to make the choice "yes, I can push to prod on Christmas Eve, and it'll totally work out for me!"
Over the next few weeks, I'm hoping to get a chance to get some holiday support horrors up on the site, in keeping with the season. Whether it's the absurd challenges of providing family tech support, the last minute pushes to production, the five alarm fires caused by a pointy-haired-bosses's incompetence, we want your tales of holiday IT woe.
So hit that submit button on the side bar, and tell us who's on Santa's naughty list this year.
Tales from the Interview: Interview Smack-Talk
In today's Tales from the Interview, our Anonymous submitter relates their experience with an anonymous company:
I had made it through the onsite, but along the way I had picked up some toxic work environment red flags. Since I had been laid off a couple months prior, I figured I wasn't in a position to be picky, so I decided I would still give it my best shot and take the job if I got it, but I'd continue looking for something better.
Then they brought me back onsite a second time for one final interview with 2 senior managers. I went in and they were each holding a printout of my resume. They proceeded to go through everything on it. First they asked why I chose the university I went to, then the same for grad school, which was fine.
Then they got to my first internship. I believe the conversation went something like this:
Manager: "How did you like it?"
Me: "Oh, I loved it!"
Manager: "Were there any negatives?"
Me: "No, not that I can think of."
Manager: "So it was 100% positive?"
Me: "Yep!"
And then they got to my first full-time job, where the same manager repeated the same line of questioning but pushed even harder for me to say something negative, at one point saying "Well, you left for (2nd company on my resume), so there must have been something negative."
I knew better than to bad-mouth a previous employer in an interview, it's like going into a first date and talking smack about your ex. But what do you do when your date relentlessly asks you to talk smack about all your exes and refuses to let the subject turn to anything else? This not only confirmed my suspicions of a toxic work environment, I also figured *they* probably knew it was toxic and were relentlessly testing every candidate to make sure they wouldn't blow the whistle on them.
That was the most excruciatingly awkward interview I've ever had. I didn't get the job, but at that point I didn't care anymore, because I was very, very sure I didn't want to work there in the long term.
I'm glad Subby dodged that bullet, and I hope they're in a better place now.
It seems like this might be some stupid new trend. I recently bombed an interview where I could tell I wasn't giving the person the answer on their checklist, no matter how many times I tried. It was a question about how I handled it when someone opposed what I was doing at work or gave me negative feedback. It felt like they wanted me to admit to more fur-flying drama and fireworks than had ever actually occurred.
I actively ask for and welcome critique on my writing, it makes my work so much better. And if my work is incorrect and needs to be redone, or someone has objections to a project I'm part of, I seek clarification and (A) implement the requested changes, (B) explain why things are as they are and offer alternate suggestions/solutions, (C) seek compromise, depending on the situation. I don't get personal about it.
So, why this trend? Subby believed it was a way to test whether the candidate would someday badmouth the employer. That's certainly feasible, though if that were the goal, you'd think Subby would've passed their ordeal with flying colors. I'm not sure myself, but I have a sneaking suspicion that the nefarious combination of AI and techbro startup culture have something to do with it.
So perhaps I also dodged a bullet: one of the many things I'm grateful for this Thanksgiving.
Feel free to share your ideas, and any and all bullets you have dodged, in the comments.
[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.CodeSOD: The Map to Your Confession
Today, Reginald approaches us for a confession.
He writes:
I've no idea where I "copied" this code from five years ago. The purpose of this code was to filter out Maps and Collections Maybe the intention was to avoid a recursive implementation by an endless loop? I am shocked that I wrote such code.
Well, that doesn't bode well, Reginald. Let's take a look at this Java snippet:
/** * * @param input * @return */ protected Map rearrangeMap(Map input) { Map retMap = new HashMap(); if (input != null && !input.isEmpty()) { Iterator it = input.keySet().iterator(); while (true) { String key; Object obj; do { do { if (!it.hasNext()) { } key = (String) it.next(); } while (input.get(key) instanceof Map); obj = input.get(key); } while (obj instanceof Boolean && ((Boolean) obj).equals(Boolean.FALSE)); if (obj != null) { retMap.put(key, obj); return retMap; } } } else { return retMap; } }The first thing that leaps out is that this is a non-generic Map, which is always a code smell, but I suspect that's the least of our problems.
We start by verifying that the input Map exists and contains data. If the input is null or empty, we return it. In our main branch, we create an iterator across the keys, before ethering a while(true) loop. So far so bad
Then we enter a pair of nested do loops. Which definitely hints that we've gone off the edge of the map here. In the inner most loop, we do a check- if there isn't a next element in the iterator, we… do absolutely nothing. Whether there is or isn't an element, we advance to the next element, risking a NoSuchElementException. We do this while the key points to an instance of Map. As always, an instanceof check is a nauseating code stench.
Okay, so the inner loop skips across any keys that point to maps, and throws an exception when it gets to the end of the list.
The surrounding loop skips over every key that is a boolean value that is also false.
If we find anything which isn't a Map and isn't a false Boolean and isn't null, we put it in our retMap and return it.
This function finds the first key that points to a non-map, non-false value and creates a new map that contains only that key/value. Which it's a hard thing to understand why I'd want that, especially since some Map implementations make no guarantee about order. And even if I did want that, I definitely wouldn't want to do that this way. A single for loop could have solved this problem.
Reginald, I don't think there's any absolution for this. Instead, my advice would be to install a carbon monoxide detector in your office, because I have some serious concerns about whether or not your brain is getting enough oxygen.
CodeSOD: Copied Homework
Part of the "fun" of JavaScript is dealing with code which comes from before sensible features existed. For example, if you wanted to clone an object in JavaScript, circa 2013, that was a wheel you needed to invent for yourself, as this StackOverflow thread highlights.
There are now better options, and you'd think that people would use them. However, the only thing more "fun" than dealing with code that hasn't caught up with the times is dealing with developers who haven't, and still insist on writing their own versions of standard methods.
const objectReplace = (oldObject, newObject) => { let keys = Object.keys(newObject) try { for (let key of keys) { oldObject[key] = newObject[key] } } catch (err) { console.log(err, oldObject) } return oldObject }It's worth noting that Object.entries returns an array containing both the keys and values, which would be a more sensible for this operation, but then again, if we're talking about using correct functions, Object.assign would replace this function.
There's no need to handle errors here, as nothing about this assignment should throw an exception.
The thing that really irks me about this though is that it pretends to be functional (in the programming idiom sense) by returning the newly modified value, but it's also just changing that value in place because it's a reference. So it has side effects, in a technical sense (changing the value of its input parameters) while pretending not to. Now, I probably shouldn't get too hung up on that, because that's also exactly how Object.assign behaves, but dammit, I'm going to be bothered by it anyway. If you're going to reinvent the wheel, either make one that's substantially worse, or fix the problems with the existing wheel.
In any case, the real WTF here is that this function is buried deep in a 15,000 line file, written by an offshore contract team, and there are at least 5 other versions of this function, all with slightly different names, but all basically doing the same thing, because everyone on the team is just copy/pasting until they get enough code to submit a pull request.
Our submitter wonders, "Is there a way to train an AI to not let people type this?"
No, there isn't. You can try rolling that boulder up a hill, but it'll always roll right back down. Always and forever, people are going to write bad code.
Error'd: Untimely
Sometimes, it's hard to know just when you are. This morning, I woke up to a Macbook that thinks it's in Paris, four hours ago. Pining for pain chocolate. A bevy of anonyms have had similar difficulties.
First up, an unarabian anonym observes "They say that visiting Oman feels like traveling back in time to before the rapid modernization of the Arab states. I just think their eVisa application system is taking this "time travel" thing a bit too far... "
Snecod, an unretired (anteretired?) anonym finds it hard to plan when the calendar is unfixed. "The company's retirement plan was having a rough time prior to Second June." Looks like the first wtf was second March.
And an unamerican anonym sent us this (uh, back in first March) "Was looking to change the cable package I have from them. Apparently my discounts are all good until 9th October 1930, and a second one looking good until 9th January 2024."
On a different theme, researcher Jennifer E. exclaimed "Those must have been BIG divorces! Guy was so baller Wikipedia couldn’t figure out when he divorced either of these women." Or so awful they divorced him continuously.
Finally, parsimonious Greg L. saved this for us. "I don't remember much about #Error!, but I guess it was an interesting day."
CodeSOD: Invalid Route and Invalid Route
Someone wanted to make sure that invalid routes logged an error in their Go web application. Artem found this when looking at production code.
if (requestUriPath != "/config:system") && (requestUriPath != "/config:system/ntp") && (requestUriPath != "/config:system/ntp/servers") && (requestUriPath != "/config:system/ntp/servers/server") && (requestUriPath != "/config:system/ntp/servers/server/config") && (requestUriPath != "/config:system/ntp/servers/server/config/address") && (requestUriPath != "/config:system/ntp/servers/server/config/key-id") && (requestUriPath != "/config:system/ntp/servers/server/config/minpoll") && (requestUriPath != "/config:system/ntp/servers/server/config/maxpoll") && (requestUriPath != "/config:system/ntp/servers/server/config/version") && (requestUriPath != "/config:system/ntp/servers/server/state") && (requestUriPath != "/config:system/ntp/servers/server/state/address") && (requestUriPath != "/config:system/ntp/servers/server/state/key-id") && (requestUriPath != "/config:system/ntp/servers/server/state/minpoll") && (requestUriPath != "/config:system/ntp/servers/server/state/maxpoll") && (requestUriPath != "/config:system/ntp/servers/server/state/version") { log.Info("ProcessGetNtpServer: no return of ntp server state for ", requestUriPath) return nil }The most disturbing part of this, for Artem, isn't that someone wrote this code and pushed it to production. It's that, according to git blame, two people wrote this code, because the first developer didn't include all the cases.
For the record, the application does have an actual router module, which can trigger logging on invalid routes.
CodeSOD: Are You Mocking Me?
Today's representative line comes from Capybara James (most recently previously). It's representative, not just of the code base, but of Goodhart's Law: when a measure becomes a target, it ceases to be a good measure. Or, "you get what you measure".
If, for example, you decide that code coverage metrics are how you're going to judge developers, then your developers are going to ensure that the code coverage looks great. If you measure code coverage, then you will get code coverage- and nothing else.
That's how you get tests like this:
Mockito.verify(exportRequest, VerificationModeFactory.atLeast(0)).failedRequest(any(), any(), any());This test passes if the function exportRequest.failedRequest is called at least zero times, with any input parameters.
Which, as you might imagine, is a somewhat useless thing to test. But what's important is that there is a test. The standards for code coverage are met, the metric is satisfied, and Goodhart marks up another win on the board.
Using an ADE: Ancient Development Environment
One of the things that makes legacy code legacy is that code, over time, rots. Some of that rot comes from the gradual accumulation of fixes, hacks, and kruft. But much of the rot also comes from the tooling going unsupported or entirely out of support.
For example, many years ago, I worked in a Visual Basic 6 shop. The VB6 IDE went out of support in April, 2008, but we continued to use it well into the next decade. This made it challenging to support the existing software, as the IDE frequently broke in response to OS updates. Even when we started running it inside of a VM running an antique version of Windows 2000, we kept running into endless issues getting projects to compile and build.
A fun side effect of that: the VB6 runtime remains supported. So you can run VB6 software on modern Windows. You just can't modify that software.
Greta has inherited an even more antique tech stack. She writes, "I often wonder if I'm the last person on Earth encumbered with this particular stack." She adds, "The IDE is long-deprecated from a vendor that no longer exists- since 2002." Given the project started in the mid 2010s, it may have been a bad choice to use that tech-stack.
It's not as bad as it sounds- while the technology and tooling is crumbling ruins, the team culture is healthy and the C-suite has given Greta wide leeway to solve problems. But that doesn't mean that the tooling isn't a cause of anguish, and even worse than the tooling- the code itself.
"Some things," Greta writes, "are 'typical bad'" and some things "are 'delightfully unique' bad."
For example, the IDE has a concept of "designer" files, for the UI, and "code behind" files, for the logic powering the UI. The IDE frequently corrupts its own internal state, and loses the ability to properly update the designer files. When this happens, if you attempt to open, save, or close a designer file, the IDE pops up a modal dialog box complaining about the corruption, with a "Yes" and "No" option. If you click "No", the modal box goes away- and then reappears because you're seeing this message because you're on a broken designer file. If you click "Yes", the IDE "helpfully" deletes pretty much everything in your designer file.
Nothing about the error message indicates that this might happen.
The language used is a dialect of C++. I say "dialect" because the vendor-supplied compiler implements some cursed feature set between C++98 and C++11 standards, but doesn't fully conform to either. It's only capable of outputting 32-bit x86 code up to a Pentium Pro. Using certain C++ classes, like std::fstream, causes the resulting executable to throw a memory protection fault on exit.
Worse, the vendor supplied class library is C++ wrappers on top of an even more antique Pascal library. The "class" library is less an object-oriented wrapper and more a collection of macros and weird syntax hacks. No source for the Pascal library exists, so forget about ever updating that.
Because the last release of the IDE was circa 2002, running it on any vaguely modern environment is prone to failures, but it also doesn't play nicely inside of a VM. At this point, the IDE works for one session. If you exit it, reboot your computer, or try to close and re-open the project, it breaks. The only fix is to reinstall it. But the reinstall requires you to know which set of magic options actually lets the install proceed. If you make a mistake and accidentally install, say, CORBA support, attempting to open the project in the IDE leads to a cascade of modal error boxes, including one that simply says, "ABSTRACT ERROR" ("My favourite", writes Greta). And these errors don't limit themselves to the IDE; attempting to run the compiler directly also fails.
But, if anything, it's the code that makes the whole thing really challenging to work with. While the UI is made up of many forms, the "main" form is 18,000 lines of code, with absolutely no separation of concerns. Actually, the individual forms don't have a lot of separation of concerns; data is shared between forms via global variables declared in one master file, and then externed into other places. Even better, the various sub-forms are never destroyed, just hidden and shown, which means they remember their state whether you want that or not. And since much of the state is global, you have to be cautious about which parts of the state you reset.
Greta adds:
There are two files called main.cpp, a Station.cpp, and a Station1.cpp. If you were to guess which one owns the software's entry point, you would probably be wrong.
But, as stated, it's not all as bad as it sounds. Greta writes: "I'm genuinely happy to be here, which is perhaps odd given how terrible the software is." It's honestly not that odd; a good culture can go a long way to making wrangling a difficult tech stack happy work.
Finally, Greta has this to say:
We are actively working on a .NET replacement. A nostalgic, perhaps masochistic part of me will miss the old stack and its daily delights.
[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.Representative Line: In the Zone
Robert R picked up a bug in his company's event scheduling app. Sometimes, events were getting reported a day off from when they actually were.
It didn't take too long to find the culprit, and as is so often the case, the culprit was handling dates with strings.
const dateAsString = event.toISOString().substr(0,10); return new Date(dateAsString);toISOString returns a "simplified" ISO8601 string, which looks like this: YYYY-MM-DDTHH:mm:ss.sssZ. The substr pops off the first ten characters, giving you YYYY-MM-DD.
The goal, as you can likely gather, is to truncate to just the date part of a date-time. And given that JavaScript doesn't have a convenient method to do that, it doesn't seem like a terrible way to solve that problem, if you don't think about what date-times contain too hard.
But there's an obvious issue here. toISOString always converts the date to UTC, converting from your local timezone to UTC. Which means when you pick off just the date portion of that, you may be off by an entire day, depending on the event's scheduled time and your local timezone.
This code doesn't simply truncate- it discards timezone information. But for an event scheduler used across the world, tracking timezones is important. You can't just throw that information away.
[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!
Error'd: Will You Still Need Me?
... when I'm eight thousand and three? Doesn't quite scan.
Old soul jeffphi hummed "It's comforting to know that I'll have health insurance coverage through my 8,030th birthday!"
Scififan Steve muttered "I asked Copilot if Tom Baker and Lalla Ward were the same age. Apparently, they have been traveling on different timelines, so Copilot claims they are the same age despite having birthdays 17 years apart. Who knew?" It's a timey-wimey thing.
An anonymous Aussie announced
"I was trying to look up a weather forecast for several
weeks in advance as I'm attending an event on Saturday 22nd
November. Apparently the Weather Channel has invented its
own calendar (or decided there are
too many days in the year), as while the 1st of November was a
Saturday and 22nd of November occur on a Saturday, the
Weather Channel has decided those dates fall on a Friday.
Rosanna is in Melbourne, Australia. Temperatures are displayed in Celsius.
" Rosanna is rated the 99th most liveable suburb of Melbourne, probably due to the aberrant calendar.
Beatrix W. wants to relocate to a more pedestrian-friendly nabe. "I was bored and looked for houses on a german real estate website. When I tried to have the distance to a house calculated I got the lovely result from the screenshot. The 99 minutes are okay when using the car but the rest is -1 minute."
Taxpayer Marlin D. is making sure he gets everything lined up for year-end. Checking his witholdings, he found that the IRS try very hard to be precise with their language. "This is from the IRS online Tax Withholding Estimator tool. I guess they really do mean *between*."
CodeSOD: Lucky Thirteen
Wolferitza sends us a large chunk of a C# class. We'll take it in chunks because there's a lot here, but let's start with the obvious problem:
private int iID0; private int iID1; private int iID2; private int iID3; private int iID4; private int iID5; private int iID6; private int iID7; private int iID8; private int iID9; private int iID10; private int iID11; private int iID12; private int iID13;If you say, "Maybe they should use an array," you're missing the real problem here: Hungarian notation. But sure, yes, they should probably use arrays. And you might think, "Hey, they should use arrays," would be an easy fix. Not for this developer, who used an ArrayList.
private void Basculer(DataTable dtFrom, DataTable dtTo) { ArrayList arrRows = new ArrayList(); int index; DataRow drNew1 = dtTo.NewRow(); DataRow drNew2 = dtTo.NewRow(); DataRow drNew3 = dtTo.NewRow(); DataRow drNew4 = dtTo.NewRow(); DataRow drNew5 = dtTo.NewRow(); DataRow drNew6 = dtTo.NewRow(); DataRow drNew7 = dtTo.NewRow(); DataRow drNew8 = dtTo.NewRow(); DataRow drNew9 = dtTo.NewRow(); DataRow drNew10 = dtTo.NewRow(); DataRow drNew11 = dtTo.NewRow(); DataRow drNew12 = dtTo.NewRow(); DataRow drNew13 = dtTo.NewRow(); DataRow drNew14 = dtTo.NewRow(); DataRow drNew15 = dtTo.NewRow(); arrRows.Add(drNew1); arrRows.Add(drNew2); arrRows.Add(drNew3); arrRows.Add(drNew4); arrRows.Add(drNew5); arrRows.Add(drNew6); arrRows.Add(drNew7); arrRows.Add(drNew8); arrRows.Add(drNew9); arrRows.Add(drNew10); arrRows.Add(drNew11); arrRows.Add(drNew12); arrRows.Add(drNew13); arrRows.Add(drNew14); arrRows.Add(drNew15); // more to come…Someone clearly told them, "Hey, you should use an array or an array list", and they said, "Sure." There's just one problem: arrRows is never used again. So they used an ArrayList, but also, they didn't use an ArrayList.
But don't worry, they do use some arrays in just a moment. Don't say I didn't warn you.
if (m_MTTC) { if (m_dtAAfficher.Columns.Contains("MTTCRUB" + dr[0].ToString())) { arrMappingNames.Add("MTTCRUB" + dr[0].ToString()); arrHeadersTexte.Add(dr[4]); arrColumnsFormat.Add(""); arrColumnsAlign.Add("1");Ah, they're splitting up the values in their data table across multiple arrays; the "we have object oriented programming at home" style of building objects.
And that's all the setup. Now we can get into the real WTF here.
if (iCompt == Convert.ToInt16(0)) { iID0 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(1)) { iID1 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(2)) { iID2 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(3)) { iID3 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(4)) { iID4 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(5)) { iID5 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(6)) { iID6 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(7)) { iID7 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(8)) { iID8 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(9)) { iID9 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(10)) { iID10 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(11)) { iID11 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(12)) { iID12 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(13)) { iID13 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } } }Remember those private iID* values? Here's how they get populated. We check a member variable called iCompt and pull the first value out of a dr variable (a data reader, also a member variable). You may have looked at the method signature and assumed dtFrom and dtTo would be used, but no- they have to purpose in this method at all.
And if you liked what happened in this branch of the if, you'll love the else:
else { if (m_dtAAfficher.Columns.Contains("MTTHRUB" + dr[0].ToString())) { arrMappingNames.Add("MTTHRUB" + dr[0].ToString()); arrHeadersTexte.Add(dr[4]); arrColumnsFormat.Add(""); arrColumnsAlign.Add("1"); if (iCompt == Convert.ToInt16(0)) { iID0 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(1)) { iID1 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(2)) { iID2 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(3)) { iID3 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(4)) { iID4 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(5)) { iID5 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(6)) { iID6 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(7)) { iID7 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(8)) { iID8 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(9)) { iID9 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(10)) { iID10 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(11)) { iID11 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(12)) { iID12 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } else if (iCompt == Convert.ToInt16(13)) { iID13 = Convert.ToInt32(dr[0]); iCompt = iCompt + 1; } } } }I can only assume that this function is called inside of a loop, though I have to wonder about how that loop exits? Maybe I'm being too generous, this might not be called inside of a loop, and the whole class gets to read 13 IDs out before it's populated. Does iCompt maybe get reset somewhere? No idea.
Honestly, does this even work? Wolferitza didn't tell us what it's actually supposed to do, but found this code because there's a bug in there somewhere that needed to be fixed. To my mind, "basically working" is the worst case scenario- if the code were fundamentally broken, it could just be thrown away. If it mostly works except for some bugs (and terrible maintainability) no boss is going to be willing to throw it away. It'll just fester in there forever.
CodeSOD: Historical Dates
Handling non-existent values always presents special challenges. We've (mostly) agreed that NULL is, in some fashion, the right way to do it, though it's still common to see some sort of sentinel value that exists outside of the expected range- like a function returning a negative value when an error occurred, and a zero (or positive) value when the operation completes.
Javier found this function, which has a… very French(?) way of handling invalid dates.
Private Function CheckOraDate(ByVal sDate As String) As String Dim OraOValDate As New DAL.PostGre.DataQuery() Dim tdate As Date If IsDate(sDate) Then Return IIf(OraOValDate.IsOracle, CustomOracleDate(Convert.ToDateTime(sDate).ToString("MM-dd-yyyy")), "'" & sDate & "'") Else '~~~ No Date Flag of Bastille Day Return CustomOracleDate(Convert.ToDateTime("07/14/1789").ToString("MM-dd-yyyy")) End If End FunctionGiven a date string, we check if it is a valid date string using IsDate. If it is, we check if our data access layer thinks the IsOracle flag is set, and if it is, we do some sort of conversion to a `CustomOracleDate", otherwise we just return the input wrapped in quotes.
All that is sketchy- any function that takes dates as a string input and then returns the date in a new format as a string always gets my hackles up. It implies loads of stringly typed operations.
But the WTF is how we handle a bad input date: we return Bastille Day.
In practice, this meant that their database system was reporting customers' birthdays as Bastille Day. And let me tell you, those customers don't look a day over 200, let alone 236.
For an extra bonus WTF, while the "happy path" checks if we should use the custom oracle formatting, the Bastille Day path does not, and just does whatever the Oracle step is every time.
.comment { border: none; }CodeSOD: Losing a Digit
Alicia recently moved to a new country and took a job with a small company willing to pay well and help with relocation costs. Overall, the code base was pretty solid. Despite the overall strong code base, one recurring complaint was that running the test suite was painfully long.
While Alicia doesn't specify what the core business is, but says: "in this company's core business, random numbers were the base of everything."
As such, they did take generating random numbers fairly seriously, and mostly used strong tools for doing that. However, whoever wrote their test suite was maybe a bit less concerned, and wrote this function:
public static Long generateRandomNumberOf(int length) { while (true) { long numb = (long)(Math.random() * 100000000 * 1000000); // had to use this as int's are to small for a 13 digit number. if (String.valueOf(numb).length() == length) return numb; } }They want many digits of random number. So they generate a random floating point, and then multiply it a few times to get a large number. If the length of the resulting number, in characters, is the desired length, we return it. Otherwise, we try again.
The joy here, of course, is that this function is never guaranteed to exit. In fact, if you request more than 15 digits, it definitely won't exit. In practice, most of the time, the function is able to hit the target length in a relative handful of iterations, but there's no guarantee for that.
Alicia was tracking down a bug in a test which called this function. So she went ahead and fixed this function so that it use a sane way to generate the appropriate amount of entropy that actually guaranteed a result. She included that change in her pull request, nobody had any comments, and it got merged in.
The unit tests aren't vastly faster than they were, but they are faster. Who knows what other surprises the test suite has in store?
CodeSOD: High Temperature
Brian (previously)found himself contracting for an IoT company, shipping thermostats and other home automation tools, along with mobile apps to control them.
Brian was hired because the previous contractor had hung around long enough for the product to launch, cashed the check, and vanished, never to be heard from again.
And let's just say that Brian's predecessor had a unique communication style.
private class NoOpHandler extends AsyncCharisticHandler { public NoOpHandler(Consumer<BluetoothGattCharacteristic> consumer) { super(null, consumer); } @Override public void Execute() { // Don't do any actual BLE Communication here, and just go straight to the callback, This // handler is just to allow people to get a callback after after a bunch of Async IO Operations // have happened, without throwing all the completion logic into the "last" async callback of your batch // since the "last" one changes. InvokeCallback(); // After this callback has been handled, recheck the queue to run any subsequent Async IO Operations OnBLEAsyncOperationCompleted(); // I'm aware this is recursive. If you get a stack overflow here, you're using it wrong. // You're not supposed to queue thousands of NoOp handlers one after the other, Stop doing it! // If you need to have code executed sequentially just, er... write a fu*king function there is // nothing special about this callback, or the thread it is called on, and you don't need to use // it for anything except getting a callback after doing a batch of async IO, and then, it runs // in the context of the last IO Completion callback, which shouldn't take ages. If you use // AsyncRunWhenCompleted() to create more of these within the callback of AsyncRunWhenCompleted // it just keeps the IO completion thread busy, which also breaks shit. // Basically, you shouldn't be using AsyncRunWhenCompleted() at all if you're not me. } }Who said bad programmers don't write comments? This bad programmer wrote a ton of comments. What's funny about this is that, despite the wealth of comments, I'm not 100% certain I actually know what I'm supposed to do, aside from not use AsyncRunWhenCompleted.
The block where we initialize the Bluetooth system offers more insight into this programmer's style.
@SuppressLint("MissingPermission") private void initializeBluetooth() { _bluetoothManager = (BluetoothManager) getSystemService(BLUETOOTH_SERVICE); _bluetoothAdapter = _bluetoothManager != null ? _bluetoothManager.getAdapter() : null; if (_bluetoothAdapter != null && _bluetoothAdapter.isEnabled()) { /* #TODO: I don't know if cancelDiscovery does anything... either good, or bad. It seems to make BLE discovery faster after * the service is restarted by android, but I don't know if it screws anything else up in the process. Someone should check into that */ _bluetoothAdapter.cancelDiscovery(); _bluetoothScanner = _bluetoothAdapter.getBluetoothLeScanner(); _scanFilters = Collections.singletonList(new ScanFilter.Builder().setServiceUuid(new ParcelUuid(BLE_LIVELINK_UUID)).build()); CreateScanCallback(); } else { // #TODO: Handle Bluetooth not available or not enabled stopSelf(); } }This is a clear example of "hacked together till it works". What does cancelDiscovery do? No idea, but we call it anyway because it seems to be faster. Should we look it up? Because yes, it sounds like calling it is correct, based on the docs. Which took me 15 seconds to find. "Someone should check into that," and apparently I am that someone.
Similarly, the second TODO seems like an important missing feature. At least a notification which says, "Hey, you need bluetooth on to talk to bluetooth devices," would go a long way.
All this is in service of an IoT control app, which seems to double as a network scanner. It grabs the name of every Bluetooth and WiFi device it finds, and sends them and a location back to a web service. That web service logs them in a database, which nobody at the company ever looks at. No one wants to delete the database, because it's "valuable", though no one can ever specify exactly how they'd get value from it. At best, they claim, "Well, every other app does it." Mind you, I think we all know how they'd get value: sell all this juicy data to someone else. It's just no one at the company is willing to say that out loud.
.comment { border: none; }Error'd: What Goes Up
As I was traveling this week (just home today), conveyances of all sorts were on my mind.
Llarry A. warned "This intersection is right near my house. Looks like it's going to be inconvenient for a while..." Keeping this in mind, I chose to take the train rather than drive.
Unfortunately US trains are restricted to plodding sublight speeds, but Mate has it better. "I love how the Swiss Federal Railways keep investing in new rolling stock... like this one that can teleport from one side of the country to the other in 0 minutes?! "
And Michael R. 's TfL seems to operate between parallel universes. "I was happy to see that the "not fitted" Northern Line train actually rolled in 2 mins later."
Daniel D. 's elevator went up but the ubiquitous screen went down. Daniel even slipped a bit of selfie into his submission. Sneaky. "This display usually features some looping video. This time it featured only desktop with bunch of scripts / *.bat files. I guess it's better when elevator's display crashes than when an actual elevator crashes?"
Joel C. 's elevator had a different fault. "The screen in my hotel's elevator is not sending quite the message they probably want."
[Advertisement] Plan Your .NET 9 Migration with Confidence
Your journey to .NET 9 is more than just one decision.Avoid migration migraines with the advice in this free guide. Download Free Guide Now!
Secure to Great Lengths
Our submitter, Gearhead, was embarking on STEM-related research. This required him to pursue funding from a governmental agency that we’ll call the Ministry of Silly Walks. In order to start a grant application and track its status, Gearhead had to create an account on the Ministry website.
The registration page asked for a lot of personal information first. Then Gearhead had to create his own username and password. He used his password generator to create a random string: D\h.|wAi=&:;^t9ZyoO
Upon clicking Save, he received an error.
Your password must be a minimum eight characters long, with no spaces. It must include at least three of the following character types: uppercase letter, lowercase letter, number, special character (e.g., !, $, % , ?).
Perplexed, Gearhead emailed the Ministry’s web support, asking why his registration failed. The reply:
Hello,The site rejects password generators as hacking attempts. You will need to manually select a password.
Ex. GHott*01
Thank you,
Support
So a long sequence of random characters was an active threat, but a 1990s-era AOL username was just fine. What developer had this insane idea and convinced other people of it? How on earth did they determine what was a "manually selected" string versus a randomly-generated one?
It seems the deciding factor is nothing more than length. If you go to the Ministry’s registration page now, their password guidelines have changed (emphasis theirs):
Must be 8-10 characters long, must contain at least one special character ( ! @ # $ % ^ & * ( ) + = { } | < > \ _ - [ ] / ? ) and no spaces, may contain numbers (0-9), lower and upper case letters (a-z, A-Z). Please note that your password is case sensitive.
Only good can come of forcing tiny passwords.
The more a company or government needs secure practices, the less good they are at secure practices. Is that a law yet? It should be.
[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!
Future Documentation
Dotan was digging through vendor supplied documentation to understand how to use an API. To his delight, he found a specific function which solved exactly the problem he had, complete with examples of how it was to be used. Fantastic!
He copied one of the examples, and hit compile, and reviewed the list of errors. Mostly, the errors were around "the function you're calling doesn't exist". He went back to the documentation, checked it, went back to the code, didn't find any mistakes, and scratched his head.
Now, it's worth noting the route Dotan took to find the function. He navigated there from a different documentation page, which sent him to an anchor in the middle of a larger documentation page- vendorsite.com/docs/product/specific-api#specific-function.
This meant that as the page loaded, his browser scrolled directly down to the specific-function section of the page. Thus, Dotan missed the gigantic banner at the top of the page for that API, which said this:
/!\ NOTE /!\ NOTE /!\ NOTE /!\ NOTE /!\ NOTE /!\ NOTE /!\ NOTE /!
This doc was written to help flesh out a user API. The features described here are all hypothetical and do not actually exist yet, don't assume anything you see on this page works in any version
/!\ NOTE /!\ NOTE /!\ NOTE /!\ NOTE /!\ NOTE /!\ NOTE /!\ NOTE /!\
On one hand, I think providing this kind of documentation is invaluable, both to your end users and for your own development team. It's a great roadmap, a "documentation driven development" process. And I can see that they made an attempt to be extremely clear about it being incomplete and unimplemented- but they didn't think about how people actually used their documentation site. A banner at the top of the page only works if you read the page from top to bottom, but documentation pages you will frequently skip to specific sections of the page.
But there was a deeper issue with the way this particular approach was executed: while the page announced that one shouldn't assume anything works, many of the functions on the page did work. Many did not. There was no rhyme or reason, to version information or other indicators to help a developer understand what was and was not actually implemented.
So while the idea of a documentation-oriented roadmap specifying features that are coming is good, the execution here verged into WTF territory. It was a roadmap, but with all the landmarks erased, so you had no idea where you actually were along the length of that road. And the one warning sign that would help you was hidden behind a bush.
Dotan asks: "WTF is that page doing on the official documentation wiki?"
And I'd say, I understand why it's there, but boy it should have been more clear about what it actually was.
Undefined Tasks
Years ago, Brian had a problem: their C# application would crash sometimes. What was difficult to understand was why it was crashing, because it wouldn't crash in response to a user action, or really, any easily observable action.
The basic flow was that the users used a desktop application. Many operations that the users wanted to perform were time consuming, so the application spun up background tasks to do them, thus allowing the user to do other things within the application. And sometimes, the application would just crash, both when the user hadn't done anything, and when all background jobs should have been completed.
The way the background task was launched was this:
seeder.RunSeeder();It didn't take too much head scratching to realize what was running every time the application crashed: the garbage collector.
RunSeeder returned a Task object, but since Brian's application treated the task as "fire and forget", they didn't worry about the value itself. But C# did- the garbage collector had to clean up that memory.
And this was running under .Net 4.0. This particular version of the .Net framework was a special, quantum realm, at least when it came to tasks. You see, if a Task raises an exception, nothing happens. At least, not right away. No one is notified of the exception unless they inspect the Task object directly. There's a cat in the box, and no one knows the state of the cat unless they open the box.
The application wasn't checking the Task result. The cat remained in a superposition of "exception" and "no exception". But the garbage collector looked at the task. And, in .Net 4.0, Microsoft made a choice about what to do there: when they opened the box and saw an exception (instead of a cat), they chose to crash.
Microsoft's logic here wasn't entirely bad; an uncaught exception means something has gone wrong and hasn't been handled. There's no way to be certain the application is in a safe state to continue. Treating it akin to undefined behavior and letting the application crash was a pretty sane choice.
The fix for Brian's team was simple: observe the exception, and choose not to do anything with it. They truly didn't care- these tasks were fire-and-forget, and failure was acceptable.
seeder.RunSeeder().ContinueWith(t => { var e = t.IsFaulted ? t.Exception : null; }); // Observe exceptions to prevent quantum crashesThis code merely opens the box and sees if there's an exception in there. It does nothing with it.
Now, I'd say as a matter of programming practice, Microsoft was right here. Ignoring exceptions blindly is a definite code smell, even for a fire-and-forget task. Writing the tasks in such a way as they catch and handle any exceptions that bubble up is better, as is checking the results.
But I, and Microsoft, were clearly on the outside in this argument. Starting with .Net 4.5 and moving forward, uncaught exceptions in background tasks were no longer considered show-stoppers. Whether there was a cat or an exception in the box, when the garbage collector observed it, it got thrown away either way.
In the end, this reminds me of my own failing using background tasks in .Net.