A Whale of a Problem
From our Anonymous submitter:
Our company creates graphs to visualize data. We have many small fish customers, but we have one whale who uses our product that is 90% of company revenue. (WTF number 1.)
So if he is not happy, it's all-hands-on deck-mode.
He complained that our APIs and charts are loading slowly for him. For 3 weeks, we've tried a TON of optimizations, including WTF 2: spinning up a special server he alone can hit.
Today, we found out that he's always complaining when he's in his car, driving from home to the office. But since he "totally has the best wifi money can buy," that isn't worth investigating.
WTF 3: thinking wifi and data are always 100% reliable in a car driving around.
Our submitter highlights one of the major pitfalls of the so-called whale client: if they're a bad client, you're in for an extra-bad time.
As I lean harder into freelancing, I'm learning to scan the waters ahead of me for potential whales. My goal is to build up multiple small, diverse income streams, because I've had my own dangerous encounters with whales in the past.
At one employer of mine, there was Facebook, who acted as if they were our new owners rather than a new customer. They'd already produced flashy marketing videos of the sorts of solutions they planned to implement with our software, showing people delighted with the results. In meetings, these things were talked up as amazing game-changers. Meanwhile, I found all the things Facebook wanted to do horribly creepy and invasive.
Even worse, Facebook began dictating how our award-winning technical support should change to accommodate their whims, up to and including having a dedicated toady—er, support rep—who did nothing but field Facebook-related tickets, similar to a technical account manager (TAM).
That was the last straw for me. I left that company before I was forced to deal with any of Facebook's crap.
My second whale sighting occurred at a startup that'd landed Porsche, far and away their biggest client ever. All of a sudden, our timeline for adding new features and fixing bugs became Porsche's honey-do list. All of a sudden, the platform frequently crashed and became unusable for everyone because it couldn't handle the amount of traffic Porsche (and their clients) hurled at it.
On the other hand, there were several times in that startup's existence when a big wad of promised funding failed to materialize. Porsche kept the business afloat and literally kept my lights on.
I find it less than ideal to be at any company's mercy. I want a world that would neither spawn whales nor millions of startups named Sploink, Dink, and Twangle that promise to bring the power of AI to your dinner fork.
Have your own epic whaling adventures? Share with us in the comments!
CodeSOD: Lint Brush Off
A few years back, C# added the concept of "primary constructors". Instead of declaring the storage for class members and then initializing them in the constructor, you can annotate the class itself with the required fields, and C# automatically generates a constructor for you. It's all very TypeScript and very Microsoft, and certainly cuts down on some boilerplate.
Esben B's team isn't really using them in many places, but they are using a linter which is opinionated about them. So this in-line constructor causes the linter to complain:
public DocumentNetworkController(ILookupClient service)The linter wants you to switch this to a primary constructor. Esben didn't want to do that, and didn't want to change the global linter configuration, and so added a pragma to disable that particular warning:
#pragma warning disable IDE0290 // Use primary constructor public DocumentNetworkController(ILookupClient service) #pragma warning restore IDE0290The linter didn't like this. It threw a new warning: that this suppression wasn't needed. Which was news to Esben, as clearly the suppression was needed if you wanted to make the warnings go away. The obvious solution was to disable the warning that you didn't need to disable the warning:
#pragma warning disable IDE0079, IDE0290 // Use primary constructor public DocumentNetworkController(ILookupClient service) #pragma warning restore IDE0290, IDE0079Except this doesn't work. These pragmas take effect on the next line, which means you can't disable IDE0079 on the same line as IDE0290 and expect it to work. Which means the final version of the code looked like this:
#pragma warning disable IDE0079 // Disable warning about not needed supression #pragma warning disable IDE0290 // Use primary constructor public DocumentNetworkController(ILookupClient service) #pragma warning restore IDE0290, IDE0079Esben writes:
So the nice recommendation to use a primary ctor ended up with 3 lines of annoying boilerplate code. Good times \o/
While yes, this is frustrating, I will say there's an element of "when the table saw keeps taking fingers off, that may be more of a you problem." I don't know the details, so I can't say, "just change the linter config or adopt its recommendation" and claim that the problem goes away, but when the tool hurts you, it's a definite sign of one of two things: it's either the wrong tool, or you're using it wrong.
CodeSOD: The JSON Template
We rip on PHP a lot, but I am willing to admit that the language and ecosystem have evolved over the years. What started as an ugly templating language is now just an ugly regular language.
But what happens when you still really want to do things with templates? Allison has inherited a Python-based, WSGI application which rejects any sort of formal routing or basic web development best practices. Their way of routing requests is simply long chains of "if condition then invokeA elif otherCondition then invokeB". Sometimes, those conditions will directly set the MIME type on the HTTP response.
They do use a templating library called Mako for generating their responses. They use it for their HTML responses, obviously. They also use it for their JSON responses, generating code like this:
{ "success": true, "items": { %for item in items_available.keys(): "${item}": ${items_available[item]}${',' if not loop.last else ''} %endfor } }The %for and matching %endfor mark the Python code off, which generates JSON via string-munging, complete with the check to make sure we're not on the last iteration of the loop.
Like so much bad code, this offers a degree of fractal wrongness. Instead of iterating over the keys and fetching the items inside the loop, you could iterate for key,value in items_available.items()- and according to the Mako docs, that for is just a regular Python for loop. That we're just outputting the contents of the dictionary is itself potentially a problem- sure, if we know the types of the dictionary, we'll know that whatever it is can be output in the body of a JSON document, but do we really think this code is using type annotations? I don't. And for a RESTful web service, I'm always going to feel weird about using a success field when ideally the HTTP status code could convey most of that information (and yes, I know there are reasons to still put status in the body, I just hate it).
Of course, the real issue is just: Python's built in JSON serialization is actually pretty advanced. And performant! You don't need any of this, you could just do something like:
return json.dumps({"success": true, "items": items_available})No templates. No formatting. No worries about how the data gets represented. Well, still worries, because JSON serialier will throw exceptions if it doesn't know what to do with a type. But then at least you get that exception on the server side and aren't sending the client a malformed document.
In any case, this is a good demonstration that you can write bad PHP in any language.
Error'd: April Showers
"RFC 1738 (and 3986) disagree" and so does Daniel D. "Reddit API has some weird app creation going on with lots of recently migrated and undocumented stuff. But having redirect URL set to localhost (or 127.0.0.1) usually works. Well, if you don't disagree with Sir Tim Berners-Lee about what URL is. Which Reddit does. hostnumber = digits "." digits "." digits "." digits". I'd file this one with all the websites that try to perform validation on email addresses, and get it wrong.
"Why aren't we getting any resumes?" wondered Fred G. "This is a snippet from a job posting. I'm sure it worked perfectly when HR tested it."
"Service required..." was Chris H.'s title for this gem. "My 2022 Chevrolet has been at the dealer for recall service for two weeks now, "waiting for parts". That doesn't stop GM from emailing every few days with a reminder that the car needs the recall service, and inviting me to schedule it at a dealer (that isn't actually a dealer) located a convenient 2500 mile drive from my home (about 200 times the distance to the dealer where the car currently sits), and providing a non-existent placeholder phone number to contact them at to schedule the recall service."
"How to subtly tell your customers that you don't wish to be contacted" explains Yuri. "The bank's staff must be wondering why no one wants to talk to them...Is it their suit's brand that is throwing everyone off? Can they blame it on COVID?"
"Bad money formatting by tax software" Adam R. complained. "I'm ashamed to admit it, but yes, I did pay Intuit money to file my taxes. This should really be a free service provided by the government, but, y'know, *lobbying*. You'd think that a business focused on tax preparation software would know how to properly format currency values, but in this case they failed to set the proper number of decimal points."
CodeSOD: Tune Out the Static
Henrik H (previously) sends us a simple representative C# line:
static void GenerateCommercilaInvoice()This is a static method which takes no parameters and returns nothing. Henrik didn't share the implementation, but this static function likely does something that involves side effects, maybe manipulating the database (to generate that invoice?). Or, possibly worse, it could be doing something with some global or static state. It's all side effects and no meaningful controls, so enjoy debugging that when things go wrong. Heck, good luck testing it. Our best case possibility is that it's just a wrapper around a call to a stored procedure.
This method signature is basically a commercila for refactoring.
Representative Line: Comment Overflow
Today, we look at a representative comment, sent to us by Nona. This particular comment was in a pile of code delivered by an offshore team.
// https://stackoverflow.com/questions/46744740/lodash-mongoose-object-id-difference/46745169"Wait," you say, "what's the WTF about a comment pointing to a Stack Overflow page. I do that all the time?"
In this case, it's because this particular comment wasn't given any further explanation. It also wasn't in a block of code that was doing anything with either lodash, Mongoose, or set differences. It was, however, repeated multiple times throughout the codebase, because the entire codebase was a pile of copy-pasta glued together with the bare minimum code to make it work.
In at least one place, the comment was probably correct and helpful. But it got swept up as part of a broader copy/paste exercise, and now is scattered through the code without any true purpose.
Turning Thirty
Eric O worked for a medical device company. The medical device industry moves slowly, relative to other technical industries. Medical science and safety have their own cadence, and at a certain point, iterating faster doesn't matter much.
Eric was working on a new feature on a system that had been in use for thirteen years. This new feature interacted with a database which stored information about racks of test tubes, and Eric's tests meant creating several entries for racks of test tubes. And that's when Eric discovered that the database only allowed thirty racks. Add any more, it would just roll right back over to one.
This was odd. The database was small- less than 40MB, even in production- and there were automatic tasks to purge old data for compliance purposes. Why a hard limit of thirty?
Eric had only been at the company for a year, so he asked one of the more senior team members, Lester. "Oh yeah, that was before my time. You should probably ask Carl."
Later that day, Eric happened to bump into Carl around the coffee maker, and asked the question. "Oh, yeah, I do vaguely remember something about that. It was in the requirements for the product. I thought it was weird, but didn't think too much about it. You should probably ask Elise, she's been here like twenty years."
Well, now it was getting curious. Eric went over to the "old building", as it was named, the original office for the company on the other side of the parking lot. Most of the offices had moved to the new building a decade earlier, and it mostly served as fabrication and storage, but a few offices remained.
Elise was on the third floor, down a poorly lit hallway, sitting in an office with water-stained acoustical tile in its ceiling. "Oh, yeah, I put that into the requirements document. It's funny, I thought it was weird too, but the system you're working on was a replacement for an older system. Our requirements were derived from those. Let me think… Irving worked on that, but he's dead, god rest him. Penny is retired. Oh, you know, Humbert is still around. He didn't work on that, but he worked on some of the systems that came before that. He's upstairs and on the other side of the building."
Eric went upstairs and to the other side of the building. The fourth floor had been last remodeled circa 1985, and the ugly industrial paint on the wall was made even uglier by the fact that someone had replaced most of the flourescent tubes with LEDs. Most. The mismatched color temperature started Eric down the path of a headache.
Humbert was in an office similar to Elise's. On his desk was a plaque commemerating 40 years of service with the company. Eric asked about the limitation, and Humbert laughed.
"You're working on the latest version of a product that initially started on an old PDP-11 running MUMPS. I mean, the first versions, anyway. We ran to desktop computers as fast as we could. I wrote a version for DOS in… oh… '86? I knew none of the facilities we worked with had more than ten or fifteen racks of tubes, and I needed somehow to limit the size of the database so it all fit on a single 5 1/4" floppy disk. I picked thirty, because it seemed like a good round number. Honestly, I'm shocked that the limit still exists."
So was Eric. There had been several ground-up-rewrites since 1986, before the one Eric maintained had been released thirteen years ago. Each one of them had chosen to maintain the same limitation, without ever considering why it existed. The rule had simply been copied, mindlessly, for 40 years.
"I'm kind of impressed," Eric said to Humbert, "in a horrified way."
"Me too, kid, me too."
CodeSOD: Good Etiquette
"Here, you're a programmer, take this over. It's business critical."
That's what Felicity's boss told her when he pointed her to a network drive containing an Excel spreadsheet. The Excel spreadsheet contained a pile of macros. The person who wrote it had left, and nobody knew how to make it work, but the macros in question were absolutely business vital.
Also, it's in French.
We'll take this one in chunks. The indentation is as in the original.
Public Sub ExporToutVersBaseDonnées(ClasseurEnCours As Workbook) Call AffectionVariables(ToutesLesCellulesNommées) Call AffectationBaseDonnées(BaseDonnées) BaseDonnées.ActivateThe procedures AffectionVariables and AffectationBaseDonnées populate a pile of global variables. "base de données" is French for database, but don't let the name fool you- anything referencing "base de données" is referencing another Excel file located on a shared server. There are, in total, four Excel files that must live on a shared server, and two more which must be in a hard-coded path on the user's computer.
Oh, and the shared server is referenced not by a hostname, but by IP address- which is why the macros were breaking on everyone's computer; the IP address changed.
Let's continue.
'Vérifier si la ligne existe déjà. If ClasseurEnCours.Sheets("DATA").Range("Num_Fichier") = 0 Then Num_Fichier = BaseDonnées.Sheets(1).Range("Dernier_Fichier").Value + 1 Insérer_Ligne: '(étiquette Goto) insérer une ligne Application.GoTo Reference:="Dernière_Ligne" Selection.EntireRow.Insert 'Copie les cellules (colonne A à colonne FI) de la ligne au-dessus de la ligne insérée. With ActiveCell .Offset(-1, 0).Range("A1:FM1").Copy 'Colle le format de la cellule précédemment copiée à la cellule active puis libère les données du presse papier .PasteSpecial .Range("A1:FM1").Value = "" 'Se repositionne au début de la ligne insérée. .Range("A1").Select End With Application.CutCopyMode = FalseUh oh, Insérer_Ligne is a label for a Goto target. Not to be confused by the Application.GoTo call on the next line- that just selects a range in the spreadsheet.
After that little landmine, we copy/paste some data around in the sheet.
That's the If side of the conditional, let's look at the else clause:
Else Cherche_Numéro_Fichier: ' Chercher la ligne ou le numéro de fichier est égale à NumFichier. While ActiveCell.Value <> Num_Fichier If ActiveCell.Row = Range("Etiquettes").Row Then GoTo Insérer_Ligne End If ActiveCell.Offset(-1, 0).Range("a1:a1").Select Wend 'Vérifier le numéro d'indice de la ligne active. If Cells(ActiveCell.Row, 165).Value <> ClasseurEnCours.Sheets("DATA").Range("Dernier_Indice") Then ActiveCell.Offset(-1, 0).Range("A1:A1").Select GoTo Cherche_Numéro_Fichier End If ActiveCell.Offset(0, 0).Range("A1:FM1").Value = "" End IfWe start with another label, and… then we have a Goto. A Goto which jumps us back into the If side of the conditional. A Goto inside of a while loop, a while loop that's marching around the spreadsheet to search for certain values in the cell.
After the loop, we have another Goto which will possibly jump us up to the start of the else block.
The procedure ends with some cleanup:
'----- ' Do some stuff on the active cell and the following cells on the column .----- BaseDonnées.Close True Set BaseDonnées = Nothing End SubI do not know what this function does, and the fact that the code is largely in a language I don't speak isn't the obstacle. I have no idea what the loops and the gotos are trying to do. I'm not even a "never use Goto ever ever ever" person; in a language like VBA, it's sometimes the best way to handle errors. But this bizarre time-traveling flow control boggles me.
"Etiquettes" is French for "labels", and it may be bad etiquette but I've got some four letter labels for this code.
Error'd: Having a Beastly Time
It's time again for a reader special, and once again it's all The Beast In Black (there must be a story to that nick, no?).
"MySQL is not better than your SQL," he pontificated, "especially when it comes to the Workbench Migration Wizard"
"Sadly," says he, "Not even gmail/chromium either."
"Updated software is available, but there are no updates!" he puzzled. "Clicking Install Now just throws that dialog right back in my face. I'm re-cursing." Zero, one, does it really make a difference?
"Questions" The Beast in Black "I do, in fact, have a question..."
One of the foundational guides to my [lyle, not bib] engineering career
was John Bentley's Programming Pearls. These are not those.
"Veni, vidi: vc. No pearls of wisdom here, just litter." says
The Beast.
[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!
CodeSOD: We'll Hire Better Contractors Next Time, We Promise
Nona writes: "this is the beginning of a 2100 line function."
That's bad. Nona didn't send us the entire JavaScript function, but sent us just the three early lines, which definitely raise concerns:
if (res.length > 0) { await (function () { return new Promise((resolve, reject) => {We await a synchronous function which retuns a promise, passing a function to the promise. As a general rule, you don't construct promises directly, you let asynchronous code generate them and pass them around (or await them). It's not a thing you never do, but it's certainly suspicious. It gets more problematic when Nona adds:
This function happens to contain multiple code repetition snippets, including these three lines.
That's right, this little block appears multiple times in the function, inside of anonymous function getting passed to the Promise.
No, the code does not work in its current state. It's unclear what the 2100 line function was supposed to do. And yes, this was written by lowest-bidder third-party contractors.
Nona adds:
I am numb at this point and know I gotta fix it or we lose contracts
Management made the choice to "save money" by hiring third parties, and now Nona's team gets saddled with all the crunch to fix the problems created by the "savings".
[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: Three Letter Acronyms, Four Letter Words
Candice (previously) has another WTF to share for us.
We're going to start by just looking at one fragment of a class defined in this C++ code: TLAflaList.
Every type and variable has a three-letter-acronym buried in its name. The specific meaning of most of the acronyms are mostly lost to time, so "TLA" is as good as any other three random letters. No one knows what "fla" is.
What drew Candice's attention was that there was a type called "list", which implies they're maybe not using the standard library and have reinvented a wheel. Another data point arguing in favor of that is that the class had a method called getNumElements, instead of something more conventional like size.
Let's look at that function:
size_t TLAflaList::getNumElements() { return mv_FLAarray.size(); }In addition to the meaningless three-letter-acronyms which start every type and variable, we're also adding on a lovely bit of hungarian notation, throwing mv_ on the front for a member variable. The variable is called "array", but is it? Let's look at that definition.
class TLAflaList { … private: TLAflaArray_t mv_FLAarray; … }Okay, that gives me a lot more nonsense letters but I still have no idea what that variable is. Where's that type defined? The good news, it's in the same header.
typedef std::vector<INtabCRMprdinvusage_t*> TLAflaArray_t;So it's not a list or an array, it's a vector. A vector of bare pointers, which definitely makes me worry about inevitable use-after-free errors or memory leaks. Who owns the memory that those pointers are referencing?
"IN" in the type name is an old company, good ol' Initrode, which got acquired a decade ago. "tab" tells us that it's meant to be a database table. We can guess at the rest.
This isn't a codebase, it's a bad Scrabble hand. It's also a trainwreck. Confusing, disorganized, and all of that made worse by piles of typedefs that hide what you're actually doing and endless acronyms that make it impossible to read.
One last detail, which I'll let Candice explain:
I started scrolling down the class definition - it took longer than it should have, given that the company coding style is to double-space the overwhelming majority of lines. (Seriously; I've seen single character braces sandwiched by two lines of nothing.) On the upside, this was one of the classes with just one public block and one private block - some classes like to ping-pong back and forth a half-dozen times.
A Hole in Your Plan
Theresa works for a company that handles a fair bit of personally identifiable information that can be tied to health care data, so for them, security matters. They need to comply with security practices laid out by a variety of standards bodies and be able to demonstrate that compliance.
There's a dirty secret about standards compliance, though. Most of these standards are trying to avoid being overly technically prescriptive. So frequently, they may have something like, "a process must exist for securely destroying storage devices before they are disposed of." Maybe it will include some examples of what you could do to meet this standard, but the important thing is that you have to have a process. This means that if you whip up a Word document called "Secure Data Destruction Process" and tell people they should follow it, you can check off that box on your compliance. Sometimes, you need to validate the process; sometimes you need to have other processes which ensure this process is being followed. What you need to do and to what complexity depends on the compliance structure you're beholden to. Some of them are surprisingly flexible, which is a polite way of saying "mostly meaningless".
Theresa's company has a process for safely destroying hard drives. They even validated it, shortly after its introduction. They even have someone who checks that the process has been followed. The process is this: in the basement, someone set up a cheap drill press, and attached a wooden jig to it. You slap the hard drive in the jig, turn on the drill, and brrrrzzzzzz- poke a hole through the platters making the drive unreadable.
There's just one problem with that process: the company recently switched to using SSDs. The SSDs are in a carrier which makes them share the same form factor as old-style spinning disk drives, but that's just a thin plastic shell. The actual electronics package where the data is stored is quite small. Small enough, and located in a position where the little jig attached to the drill guarantees that the drill won't even touch the SSD at all.
For months now, whenever a drive got decommissioned, the IT drone responsible for punching a hole through it has just been drilling through plastic, and nothing else. An unknown quantity of hard drives have been sent out for recycling with PII and health data on them. But it's okay, because the process was followed.
The compliance team at the company will update the process, probably after six months of meetings and planning and approvals from all of the stakeholders. Though it may take longer to glue together a new jig for the SSDs.
CodeSOD: Non-cogito Ergo c_str
Tim (previously) supports a relatively ancient C++ application. And that creates some interesting conundrums, as the way you wrote C++ in 2003 is not the way you would write it even a few years later. The standard matured quickly.
Way back in 2003, it was still common to use C-style strings, instead of the C++ std::string type. It seems silly, but people had Strong Opinions™ about using standard library types, and much of your C++ code was probably interacting with C libraries, so yeah, C-strings stuck around for a long time.
For Tim's company, however, the migration away from C-strings was in 2007.
So they wrote this:
if( ! strncmp( pdf->symTabName().c_str(), prefix.c_str(), strlen( prefix.c_str() ) ) ) { // do stuff }This is doing a "starts with" check. strncmp, strlen are both functions which operate on C-strings. So we compare the symTabName against the prefix, but only look at as many characters as are in the prefix. As is common, strncmp returns 0 if the two strings are equal, so we negate that to say "if the symTabName starts with prefix, do stuff".
In C code, this is very much how you would do this, though you might contemplate turning it into a function. Though maybe not.
In C++, in 2007, you do not have a built-in starts_with function- you have to wait until the C++20 standard for that- but you have some string handling functions which could make this more clear. As Tim points out, the "correct" answer is: if(pdf->symTabName().find(prefix) != 0UL). It's more readable, it doesn't involve poking around with char*s, and also isn't spamming that extra whitespace between every parenthesis and operator.
Tim writes: "String handling in C++ is pretty terrible, but it doesn't have to be this terrible."
Error'd: Youth is Wasted on the Junge
"My thoughts exactly" muttered Jason H. "I was in a system that avoids check constraints and the developers never seemed to agree to a T/F or Y/N or 1/0 for indicator columns. All data in a column will use the same pattern but different columns in the same table will use different patterns so I'm not sure why I was surprised when I came across the attached. Sort the data descending and you have the shorthand for what I uttered." How are these all unique?
"I'd better act quickly!" Hugh Scenic almost panicked. "This Microsoft Rewards offer might expire (in just under 74 years)!"
"Copy-copy-copy" repeated Gordon. "Not sure I want the team to be in touch - my query might be best left unanswered."
"Was Comcast's episode guide data hacked by MAGA?" Barry M. wondered. "This is not the usual generic description of Real Time."
"Holiday Workshop for Children learning how to write web pages, apparently," notes self-named Youth P. "You need a new category - because it is no error to involve young people in a web design workshop during their holidays. A little bit of a surprise was that it will happen in a local museum, and that children between 8 and 12 are the target audience - should they really already think about their work future?"
[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!
CodeSOD: Take a Percentage
When looking at the source of a major news site, today's anonymous submitter sends us this very, very mild, but also very funny WTF:
<div class="g-vhs g-videotape g-cinemagraph" id="g-video-178_article_slug-640w" data-type="videotape" data-asset="https://somesite.com/videos/file.mp4" data-cinemagraph="true" data-allow-multiple-players="true" data-vhs-options='{"ratio":"560:320"}' style="padding-bottom: 57.14285714285714%">Look, I know that percentage was calculated by JavaScript, or maybe the backend, or maybe calculated by a CSS pre-processor. No human typed that. There's nothing to gain by adding a rounding operation. There's nothing truly wrong with that line of code.
But I can't help but think about the comedic value in controlling your page layout down to sub-sub-sub-sub-sub-sub-sub-sub-pixel precision. This code will continue to have pixel accuracy out to screens with quadrillions of pixels, making it incredibly future proof.
It's made extra funny by calling the video player VHS and suggesting the appropriate ratio is 560 pixels by 320- which is not quite 16:9, but is a frequent letterbox ratio on DVD prints of movies.
In any case, I eagerly await a 20-zetta-pixel displays, so I can read the news in its intended glory.
CodeSOD: Two Conversions
The father of the "billion dollar mistake" left us last month. His pointer is finally null. Speaking of null handling, Randy says he was "spelunking" through his codebase and found this pair of functions, which handles null.
public String getDataString() { if (dataString == null) { return Constants.NOT_AVAILABLE; } return asUnicode(dataString); }I assume Constants.NOT_AVAILABLE is an empty string, or something similar. It's reasonable to convert a null into something like that. I don't know where this fits in the overall stack; I'm of the mind that you should retain the null until you absolutely can't anymore; like it or not, a null means something different than an empty string. Or, if we're going that far, we should be talking about using Optional or nullable types.
But that call to asUnicode seems curious. What's happening in there?
private String asUnicode(String rawValue) { if (rawValue != null) { return HtmlUtils.htmlUnescape(rawValue); } else { return rawValue; } }This function, which is only called from getDataString, checks for a null. Which we know it won't get, but it checks anyway. If it isn't null, we unescape it. If it is null, we return that null.
Well, I suppose that fits my rule of "retaining the null", but like, in the worst way you could do it. It honestly feels like, if the "swap the null for an empty string" happens anywhere, it should happen here. If I ask for the unescaped version of a null string, an empty string is a reasonable return. That makes more sense that doing it in a property getter.
This code isn't a trainwreck, but it makes things confusing. Maybe it's because I've been doing a lot of refactoring lately, but confusing code with unclear boundaries between functions is a raw nerve for me right now, and this particular example is stepping on that nerve.
While we're talking about unclear boundaries, I object to the idea that this class is storing dataString as an HTML escaped string that we unescape any time we want to look at it. It implies that there's some confusion about which representation is the canonical one: unescaped or escaped. We should store the canonical one, which I think is unescaped. We should only escape it at the point where we're sending it into an HTML document (or similar). Convert at the module boundary, not just any time you want to look at a string.
CodeSOD: Proper Property Validation
Tim H inherited some code which has objects that have many, many properties properties on them. Which is bad. That clearly has no cohesion. But it's okay, there's a validator function which confirms that object is properly populated.
The conditions and body of the conditionals have been removed, so we can see what the flow of the code looks like.
if (...) { if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } else if (...) { } } else { // default }It's important to note that this conditional doesn't validate every property on the object. Just most of them.
Even with autocomplete I feel like this is going to make you wear out your "{" key.
.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.CodeSOD: The Update Route
Today's anonymous submission is one of the entries where I look at it and go, "Wait, that's totally wrong, that could have never worked." And then I realize, that's why it was submitted: it was absolutely broken code which got to production, somehow.
Collection.updateOne(query, update, function(err, result, next)=>{ if(err) next(err) ... })So, Collection.updateOne is an API method for MongoDB. It takes three parameters: a filter to find the document, an update to perform on the document, and then an object containing other parameters to control how that update is done.
So this code is simply wrong. But it's worse than that, because it's wrong in a stupid way.
When creating routes using ExpressJS, you define a route and a callback to handle the route. The callback takes a few parameters: the request the browser sent, the result we're sending back, and a next function, which lets you have multiple callbacks attached to the same route. By invoking next() you're passing control to the next callback in the chain.
So what we have here is either an absolute brain fart, or more likely, a find-and-replace failure. A route handling callback got mixed in with database operations (which, as an aside, if your route handling code is anywhere near database code, you've also made a horrible mistake). The result is a line of code that doesn't work. And then someone released this non-working code into production.
Our submiter writes:
This blew up our logs today, has been in the code since 2019. I removed it in a handful of other places too.
Which raises the other question: why didn't this blow up the logs earlier?
Error'd: Clever domain name here
An anonymous cable-puller wrote "Reading a long specification manual. The words "shall" and "shall not" have specific meaning, and throughout the document are in bold italic. Looks like someone got a bit shall-ow with their search-and-replace skills."
Picki jeffphi attends to details. "Apparently this recruiter doesn't have a goal or metric around proper brace selection and matching." You're hired.
UGG.LI admins highlighted "even KFC hat Breakpoints deployed in Prod now ..." I wanted to say something funny about Herren Admins' Handle but reminded myself of John Scalzi's quote about the failure case of smartass so I refrained. You might be funnier than I.
Smarter still, Steve says "A big company like Google surely has a huge QA staff and AI bots to make sure embarrassing typos don't slip through, right? You wouldn't want to damage you reputation..."
I'll bet Pascal didn't expect this, eh? "Delivered, but On the way, Searching for a driver, but Asdrubal"
CodeSOD: One Case
I feel like we've gotten a few SQL case statement abuses recently, but a properly bad one continues to tickle me. Ken C sends us one that, well:
SELECT CASE h.DOCUMENTTYPE WHEN 2 THEN 3 WHEN 3 THEN 4 WHEN 4 THEN 5 WHEN 5 THEN 6 WHEN 6 THEN 7 WHEN 7 THEN 8 ELSE h.DOCUMENTTYPE END AS DocumentType, h.DOCNMBR AS DocNmbr, h.FULLPOLICY AS FullPolicy, h.BATCHID AS BatchId, h.OrigBatchId, h.UPDATEDDATE AS UpdatedDate, h.CUSTOMERNO AS CustomerNo, h.PROJECTID AS ProjectID, h.AMOUNT AS AmountOn one hand, I can't say "just add one", because clearly sometimes they don't want to add one. On the other hand, there's an element of looking at this and knowing: well, something absolutely stupid has happened here. Maybe it was two disjoint databases getting merged. Maybe it was just once upon a time, when this database was a spreadsheet, the user responsible did a weird thing. Maybe some directive changed the document type numbering. Hell, maybe that ELSE clause never gets triggered, and we actually could just do arithmetic.