Skip to main content

Representative Line: The Batch Managing Batch File

7 hours 18 minutes ago

Carl was debugging a job management script. The first thing that caught his attention was that the script was called file.bat. They were running on Linux.

The second thing he noticed, was that the script was designed to manage up to 999 jobs, and needed to simply roll job count over once it exceeded 999- that is to say, job 1 comes after job 999.

Despite being called file.bat, it was in fact a Bash script, and thus did have access to the basic mathematical operations bash supports. So while this could have been done via some pretty basic arithmetic in Bash, doing entirely in Bash would have meant not using Awk. And if you know how to use Awk, why would you use anything but Awk?

njobno=`echo $jobno | awk '{if ($0<999) {print $0 + 1} else { print 1 }}'`

As Carl writes: "I don't mind the desire to limit job count by way of mod(1000) but what an implementation!"

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.
Remy Porter

Error'd: Domino Theory

3 days 7 hours ago

Cool cat Adam R. commented "I've been getting a bunch of messages from null in my WhatsApp hockey group."

 

Shockingly big-handed Orion S. exclaimed "When I shared this with the sender, she offered to send me an (inf) next!" Lucky Orion didn't actually receive an (expl).

 

Mike S. mused "I've heard of Paris, Texas, but NULL, Texas...that's a new one. (from Monster.com)" Texas is a big place, Mike. There's bound to be at least one of everything there.

 

Some time ago, a couple of readers let us know about a major restaurant that had flubbed their website. We didn't run it at that time but since we're doing nulls today, chew on this thought: if Error'd doesn't hold the powerful multinationals to account, what will stop all the rest of the dominos from falling in a terrible pizza chain reaction?
Hyphenated Lincoln K-C reported "No redaction needed... nully I'm not null." and Emily bemoaned "This pizza is making me feel empty inside..."

 

Finally on this Friday, an anonymous dig at the software we all love/hate to hate/love. "Just to be clear, I have absolutely no trust issues with the null gadget. However, I don't see the 'Approve Access' button anywhere."

 

[Advertisement] ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.
Lyle Seaman

Representative Line: A Date Next Month

4 days 7 hours ago

We all know the perils of bad date handling, and Claus was handed an annoying down bug in some date handling.

The end users of Claus's application do a lot of work in January. It's the busiest time of the year for them. Late last year, one of the veteran users raised a warning: "Things stop working right in January, and it creates a lot of problems."

Unfortunately, that was hardly a bug report. "Things stop working?" What does that even mean. Claus's team made a note of it, but no one thought too much about it, until January. That's when a flood of tickets came in, all relating to setting a date.

Now, this particular date picker didn't default to having dates, but let the users pick values like "Start of last month", or "Same day last year", or "Same day next month". And the trick to the whole thing was that sometimes setting dates worked just fine, sometimes it blew up, and it seemed to vary based on when you were setting the dates and what dates you were setting.

And let's take a look at one example of how this was implemented, which highlights why January was such a problem:

startDate = startDateString == "Start of last month" ? new DateTime(today.Year, today.Month-1, 1) : startDate;

They construct a new date out of the current date, by subtracting one from the Month of the current date. Which would never work for January.

And this pattern was used for all of their date handling. So "Same Day Next Month" worked fine- unless the current date was near the end of January, since they just added one to the month. Of course, "Same day next year" blows up on February 29th. And "Same day next/last week" works by adding or subtracting 7 from the day, which does not play nicely with month boundaries.

The fix was obvious: rewrite all of the code to do proper date arithmetic. The nuisance to the whole thing was that there were just a lot of "default" date arrangements that the software supported, so it was a surprising amount of effort to fix. And, given the axiom that "every change breaks somebody's workflow", there were definitely a few users who objected to the fixes- they were using the broken behavior as part of their workflow (and perhaps an excuse for why certain scheduling arrangements were "forbidden").

As a bonus complaint, I will say, I don't love the use of string literals to represent the different kinds of operations. It's certainly not the WTF here, but I am going to complain about it anyway.

[Advertisement] ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.
Remy Porter

A Refreshing Change

5 days 7 hours ago

Dear Third-Party API Support,

You're probably wondering how and why your authorization server has been getting hammered every single day for more than 4 years. It was me. It was us—the company I work for, I mean. Let me explain.

I’m an Anonymous developer at Initech. We have this one mission-critical system which was placed in production by the developer who created it, and then abandoned. Due to its instability, it received frequent patches, but no developer ever claimed ownership. No one ever took on the task of fixing its numerous underlying design flaws.

About 6 months ago, I was put in charge of this thing and told to fix it. There was no way I could do it on my own; I begged management for help and got 2 more developers on board. After we'd released our first major rewrite and fix, there were still a few lingering issues that seemed unrelated to our code. So I began investigating the cause.

This system has 10+ microservices which are connected like meatballs buried deep within a bowl of spaghetti that completely obscures what those meatballs are even doing. Untangling this code has been a chore in and of itself. Within the 3 microservices dedicated to automated tasks, I found a lot of random functionality ... and then I found this!

See, our system extracts data from your API. It takes the refresh token, requests a new access token, and saves it to our database. Our refresh token to this system is only valid for 24 hours; as soon as we get access, we download the data. Before we download the data, we ensure we have a valid access token by refreshing it.

One of our microservice's pointless jobs was to refresh the access token every 5, 15, and 30 minutes for 22 of the 24 hours we had access to it. It was on a job timer, so it just kept going. Every single consent for that day kept getting refreshed, over and over.

Your auditing tools must not have revealed us as the culprit, otherwise we should've heard about this much sooner. You've probably wasted countless hours of your lives sifting through log files with a legion of angry managers breathing down your necks. I’m writing to let you know we killed the thing. You won’t get spammed again on our watch. May this bring you some closure.

Sincerely,

A Developer Who Still Cares

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.
Ellis Morning

CodeSOD: The Bob Procedure

6 days 7 hours ago

Joe recently worked on a financial system for processing loans. Like many such applications, it started its life many, many years ago. It began as an Oracle Forms application in the 90s. By the late 2000s, Oracle was trying to push people away from forms into their newer tools, like Oracle ApEx (Application Express), but this had the result of pushing people out of Oracle's ecosystem and onto their own web stacks.

The application Joe was working on was exactly that. Now, no one was going to migrate off of an Oracle database, especially because 90% of their business logic was wired together out of PL/SQL packages. But they did start using Java for developing their UI, and then at some other point, started using Liquibase for helping them maintain and manage their schema.

The thing about a three decade old application is that it often collects a lot of kruft. For example, this procedure:

CREATE OR REPLACE PROCEDURE BOB(p_str IN VARCHAR2) AS BEGIN dbms_output.put_line(p_str); END;

Bob here is just a wrapper around a basic print statement. Presumably, the original developer- I'm gonna go out on a limb and guess that dev was also named Bob- wanted a convenient debugging function while tracking down an error, and threw this into the codebase.

As WTFs go, the function isn't that interesting. But you know what is interesting? Its legacy. This procedure is older than any source control history the company has, which means it's been in the codebase for at least twenty years, but probably much longer. Nothing invokes it. It's survived a migration into SVN then into Git, countless database upgrades, a few (unfortunate) disaster recovery events, and still sits there, waiting to help Bob debug a problem in the database.

Joe writes:

I still don't know who Bob is. Nobody knew who Bob was when I asked around. But Bob, if you're still out there, just know that you are now cemented into a part of the software that hundreds of companies used to manage loans.

[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!
Remy Porter

CodeSOD: The File Transfer

1 week ago

SQL Server Integration Services is Microsoft's ETL tool. It provides a drag-and-drop interface for describing data flows from sources to sinks, complete with transformations and all sorts of other operations, and is useful for migrating data between databases, linking legacy mainframes into modern databases, or doing what most people seem to need: migrating data into Excel spreadsheets.

It's essentially a full-fledged scripting environment, with a focus on data-oriented operations. The various nodes you can drag-and-drop in are database connections, queries, transformations, file system operations, calls to stored procedures, and so on. It even lets you run .NET code inside of SSIS.

Which is why Lisa was so surprised that her predecessor had a "call stored procedure" node called "move file". And more than that, she was surprised that the stored procedure looked like this:

if (@doDelete = 1) begin set @cmdText = 'mv -f ' + @pathName + @FileName + @FileExt + ' ' + @pathName + 'archive\' + @FileName + @FileExt + '.archive' end else begin set @cmdText = 'cp -f ' + @pathName + @FileName + @FileExt + ' ' + @pathName + 'archive\' + @FileName + @FileExt + '.archive' end insert into #cmdOutput exec @cmdResult = master.dbo.xp_cmdshell @cmdText

This stored procedure was called from SSIS, which again, I want to stress, has the functionality to do this without calling a stored procedure. But this approach offers us a few unique "advantages".

First, it requires xp_cmdshell be enabled. This particular stored procedure is disabled by default, since it allows a user inside of SQL Server to invoke shell commands. Microsoft disables this by default, because it's a gaping security hole. Any security scanning tool you may run against your server will call it out as a big red flag. You're one SQL Injection attack away from an old rm -rf /.

Which, speaking of rm, you'll note the command strings they build to execute. mv and cp. Now, SQL Server can run on Linux, but this instance wasn't. No, the person responsible for this stored procedure also installed GNU Core Utils on Windows, just so they could have mv and cp to invoke from this stored procedure. Even better, they didn't document this dependency, so the first time someone tried to migrate the database to new hardware, this functionality broke and no one knew why.

At least the migration gave them a chance to update their SSIS packages to use the "File Transfer Task" instead of this stored procedure. But don't worry, there were plenty of other stored procedures using xp_cmdshell.

[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.
Remy Porter

Error'd: Yes We Have No Bananas

1 week 3 days ago

There is fire sale on "Test In Production" incidents this week. (Ok, truth is that some of them are a little crusty and stale so we just mark them way down and push them all out at a loss). To be completely fair, testing in production is vitally important. If you didn't do that, the only way you'd know if something is broken is when one of your paying customers finds out. I call that testing in production the expensive way. The only WTFy thing about these is that when you test in production, your customers shouldn't stumble across the messes.

"We don't often test, but when we do it's always in production" snarked Brad W. unfairly. "My phone gave its default alert noise mixed with... some sound that made it seem like the phone was damaged. This was the alert that appeared. "

 

Next up, a smoking hot deal fresh off the press from Keith R. who gloated "Looks like Firestone Auto Care is testing in Production, and it's not going well! That's a lot more than 15/25/90 characters!"

 

Followed by a slightly older option from Crunchyroll, supplied by Miquel B. who reasoned "It's nice to see Crunchyroll likes to test in production every once in a while. Last time I caught them doing so, the live date happened to be the same week as the current week, but this time, I needed to look back a week on the release schedule to spot it. If I didn't have a reason to look back a week, I would not have noticed it this time."

 

Cole T. tersely typed "SL TEST DEFAULT".

 

"I misspelled my search term and found this pricey test item on a live webstore," reported Mark T.

 

Chemist Pieter learned about banana esters in high school chem class, but didn't expect banana testers in his daily news.

 

Reaching back a bit, Paul D. noted of Engadget that "Either someone was testing in production or was submitting before finishing their article. The article was retrieved via my RSS reader and I saw it a few days later. At that moment, the original post did not exist anymore."

 

And going even farther way back, we got a couple of simultaneous submissions about a leaky production test by Amazon Prime video, one from someone styled Larry and again by a Bastian H.

 

Finally, honestly, this Error'd at Honest.com is probably well past its best-by date. But we're throwing it in to the bag at no charge. Seth N. thought "Honest company got a little too honest with this pop up modal after logging in" but we think honesty is just the best policy.

 

[Advertisement] ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.
Lyle Seaman

CodeSOD: A JSON Serializer

1 week 4 days ago

Carol sends us today's nasty bit of code. It does the thing you should never do: serializes by string munging.

public string ToJSON() { double unixTimestamp = ConvertToMillisecondsSinceEpoch(time); string JSONString = "{\"type\":\"" + type + "\",\"data\":{"; foreach (string key in dataDict.Keys) { string value = dataDict[key].ToString(); string valueJSONString; double valueNumber; bool valueBool; if (value.Length > 2 && value[0].Equals('(') && value[value.Length - 1].Equals(')')) //tuples { char[] charArray = value.ToCharArray(); charArray[0] = '['; charArray[charArray.Length - 1] = ']'; if (charArray[charArray.Length - 2].Equals(',')) charArray[charArray.Length - 2] = ' '; valueJSONString = new string(charArray); } else if ((value.Length > 1 && value[0].Equals('{') && value[value.Length - 1].Equals('}')) || (double.TryParse(value, out valueNumber))) //embedded json or numbers { valueJSONString = value; } else if (bool.TryParse(value, out valueBool)) //bools { valueJSONString = value.ToLower(); } else //everything else is a string { valueJSONString = "\"" + value + "\""; } JSONString = JSONString + "\"" + key + "\":" + valueJSONString + ","; } if (dataDict.Count > 0) JSONString = JSONString.Substring(0, JSONString.Length - 1); JSONString = JSONString + "},\"time\":" + unixTimestamp.ToString() + "}"; return JSONString; }

Now, it's worth noting, C# already has some pretty usable JSON serialization built-ins. None of this code needs to exist in the first place. It's parsing across a dictionary, but that dictionary is itself constructed by copying properties out of an object.

What's fun in this is because everything's shoved into the dictionary and then converted into strings (for the record, the dictionary stores objects, not strings) the only way they have for sniffing out the type of the input is to attempt to parse the input.

If it starts and ends with a (), we convert it into an array of characters. If it starts and ends with {} or parses as a double, we just shove it into the output. If it parses as a boolean, we convert it to lower case. Otherwise, we throw ""s around it and put that in the output. Notably, we don't do any further escaping on the string, which means strings containing " could do weird things in our output.

The final delight to this is the realization that their method of handling appending will include an extra comma at the end, which needs to be removed. Hence the if (dataDict.Count > 0) check at the end.

As always, if you find yourself writing code to generate code, stop. Don't do it. If you find you actually desperately need to start by building a syntax tree, and only then render it to text. If you find yourself needing to generate JSON specifically, please, I beg you, just get a library or use your language's built-ins.

[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!
Remy Porter

A Unique Mistake

1 week 5 days ago

Henrik spent too many hours, staring at the bug, trying to understand why the 3rd party service they were interacting with wasn't behaving the way he expected. Henrik would send updates, and then try and read back the results, and the changes didn't happen. Except sometimes they did. Reads would be inconsistent. It'd work fine for weeks, and then suddenly things would go off the rails, showing values that no one from Henrik's company had put in the database.

The vendor said, "This is a problem on your side, clearly." Henrik disagreed.

So Henrik went about talking over the problem with his fellow devs, working with the 3rd party support, and building test cases which could reproduce the results reliably. It took many weeks of effort, but by the end, he was confident he could prove it was the vendor's issue.

"Hey," Henrik said, "I think these tests pretty convincingly show that it's a problem on your side. Let me know if the tests highlight anything for you."

The bug ticket vanished into the ether for many weeks. Eventually, the product owner replied. Their team had diagnosed the problem, and the root cause was that sometimes the API would get confused about record ownership and identity. It was a tricky problem to crack, but the product owner's developers had come up with a novel solution to resolve it:

Actually we could add a really unique id instead which would never get repeated, even across customers, it would stay the same for the entity and never be reused

And thus, the vendor invented primary keys and unique identifiers. Unfortunately, the vendor was not E.F. Codd, the year was not 1970, primary keys had been invented already, and in fact were well understood and widely used. But not, apparently, by this vendor.

[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!
Remy Porter

Representative Line: Listing Off the Problems

1 week 6 days ago

Today, Mike sends us a Java Representative Line that is, well, very representative. The line itself isn't inherently a WTF, but it points to WTFs behind it. It's an omen of WTFs, a harbinger.

ArrayList[] data = new ArrayList[dataList.size()];

dataList is an array list. This line creates an array of arraylists, equal in length to dataList. Why? It's hard to say for certain, but the whiff I get off it is that this is an attempt to do "object orientation by array(list)s." Each entry in dataList is some sort of object. They're going to convert each object in dataList into an arraylist of values. This also either is old enough to predate Java generics (which is its own WTF), or explicitly wants a non-generic version so that it can do this munging of values into an array.

Mike provided no further explanation, so that's all speculation. But what is true is that when I see a line like that, my programmer sense starting tingling- something is wrong, even if I don't quite know what.

[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.
Remy Porter

CodeSOD: A Monthly Addition

2 weeks ago

In the ancient times of the late 90s, Bert worked for a software solutions company. It was the kind of company that other companies hired to do software for them, releasing custom applications for each client. Well, "each" client implies more than one client, but in this company's case, they only had one reliable client.

One day, the client said, "Hey, we have an application we built to handle scheduling helpdesk workers. Can you take a look at it and fix some problems we've got?" Bert's employer said, "Sure, no problem."

Bert was handed an Excel file, loaded with VBA macros. In the first test, Bert tried to schedule 5 different workers for their shifts, only to find that resolving the schedule and generating output took an hour and a half. Turns out, "being too slow to use" was the main problem the client had.

Digging in, Bert found code like this:

IF X = 0 THEN Y = 1 ELSE IF X = 1 THEN Y = 2 ELSE IF X = 2 THEN Y = 3 ELSE IF X = 3 THEN Y = 4 ELSE IF X = 4 THEN Y = 5 ELSE IF X = 5 THEN Y = 6 ELSE IF X = 6 THEN Y = 7 ELSE IF X = 7 THEN Y = 8 ELSE IF X = 8 THEN Y = 9 ELSE IF X = 9 THEN Y = 10 ELSE IF X = 10 THEN Y = 11 ELSE IF X = 11 THEN Y = 12 END IF END IF END IF END IF END IF END IF END IF END IF END IF END IF

Clearly it's to convert zero-indexed months into one-indexed months. This, you may note, could be replaced with Y = X + 1 and a single boundary check. I hope a boundary check is elsewhere in this code. because otherwise this code may have problems in the future. Well, it has problems in the present, but it will have problems in the future too.

Bert tried to explain to his boss that this was the wrong tool for the job, that he was the wrong person to write scheduling software (which can get fiendishly complicated), and the base implementation was so bad it'd likely be easier to just junk it.

The boss replied that they were going to keep this customer happy to keep money rolling in.

For the next few weeks, Bert did his best. He managed to cut the scheduling run time down to 30 minutes. This was a significant enough improvement that the boss could go back to the client and say, "Job done," though it was not significant enough, so no one ever actually used the program. The whole thing was abandoned.

Some time later, Bert found out that the client had wanted to stop paying for custom software solutions, and had drafted one of their new hires, fresh out of school, into writing software. The new hire did not have a programming background, but instead was part of their accounting team.

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.
Remy Porter

Error'd: Neither Here nor There

2 weeks 3 days ago

... or maybe I should have said both here and there?

The Beast in Black has an equivocal fuel system. "Apparently, the propane level in my storage tank just went quantum, and even the act of observing the level has not collapsed the superposition of more propane and less propane. I KNEW that the Copenhagen Interpretation couldn't be objectively correct."

 

Darren thinks YouTube can't count, complaining "I was checking YouTube Studio to see how my videos were doing (not great was the answer), but when I put them in descending order of views it would seem that YouTube is working off a different interpretation of how number work." I'm not sure whether I agree or not.

 

"The News from GitLab I was waiting for :-D" reports Christian L.

 

"Daylight Saving does strange things to our weather," observes an anonymous ned. "The clocks go forwards tonight. The temperature drops and the wind gets interesting."

 

"I guess they should have used /* */. Or maybe //. Could it be --? Or would # have impounded the text?" speculated B.J.H. Ironically, B.J.'s previous attempt failed with a 500 error, which I insist is always a server bug. B.J. speculated it was because his proposed subject (<!-- You can't read this! -->) provoked a parse error."

 

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.
Lyle Seaman

Tales from the Interview: Tic Tac Whoa

2 weeks 4 days ago

Usually, when we have a "Tales from the Interview" we're focused on bad interviewing practices. Today, we're mixing up a "Tales" with a CodeSOD.

Today's Anonymous submitter does tech screens at their company. Like most companies do, they give the candidate a simple toy problem, and ask them to solve it. The goal here is not to get the greatest code, but as our submitter puts it, "weed out the jokers".

Now our submitter didn't tell us what the problem was, but I don't have to know what the problem was to understand that this is wrong:

int temp1=i, temp2=j; while(temp1<n&&temp2<n&&board[temp1][temp2]==board[i][j]){ if(temp1+1>=n||temp1+2>=n) break; if(board[temp1][temp2]==board[temp1][temp2+1]==board[temp1][temp2+2]) points++; ele break; temp2++; }

As what is, in essence, a whiteboard coding exercise, I'm not going to mark off for the typo on ele (instead of else). But there's still plenty "what were you thinking" here.

From what I can get just from reading the code, I think they're trying to play tic-tac-toe. I'm guessing, but that they check three values in a column makes me think it's tic-tac-toe. Maybe some abstracted version, where the board is larger than 3x3 but you can score based on any run of length 3?

So we start by setting temp1 and temp2 equal to i and j. Then our while loop checks: are temp1 and temp2 still on the board, and does the square pointed at by them equal the square pointed at by i and j.

At the start of our loop, we have a second check, which is testing for a read-ahead; ensuring that our next check doesn't fall off the boundaries of the array. Notably, the temp1 part of the check isn't really used- they never finished handling the diagonals, and instead are only checking the vertical column on the next. Similarly, temp2 is the only thing incremented in the loop, never temp1.

All in all, it's a mess, and no, the candidate did not receive an offer. What we're left with is some perplexing and odd code.

I know this is verging into soapbox territory, but I want to have a talk about how to make tech screens better for everyone. These are things to keep in mind if you are administering one, or suffering through one.

The purpose of a tech screen is to inspire conversation. As a candidate, you need to talk through your thought process. Yes, this is a difficult skill that isn't directly related to your day-to-day work, but it's still a useful skill to have. For the screener, get them talking. Ask questions, pause them, try and take their temperature. You're in this together, talk about it.

The screen should also be an opportunity to make mistakes and go down the wrong path. As the candidate's understanding of the problem develops, they'll likely need to go backwards and retrace some steps. That's good! As a candidate, you want to do that. Be gracious and comfortable with your mistakes, and write code that's easy to fix because you'll need to. As a screener, you should similarly be gracious about their mistakes. This is not a place for gotchas or traps.

Finally, don't treat the screen as an "opportunity to weed out jokers". It's so tempting, and yes, we've all had screens with obviously unqualified candidates. It sucks for everybody. But if you're in the position to do a screen, I want to tell you one mindset hack that will make you a better interviewer: you are not trying to filter out candidates, you are gathering evidence to make the best case for this candidate.

Your goal, in administering a technical screen, is to gather enough evidence that you can advocate for this candidate. Your company clearly needs the staffing, and they've gotten this far in the interview process, so let's assume it's not a waste of everyone's time.

Many candidates will not be able to provide that evidence. I'm not suggesting you override your judgment and try and say "this (obviously terrible) candidate is great, because (reasons I stretch to make up)." But you want to give them every opportunity to convince you they're a good fit for the position, you want to dig for evidence that they'll work out. Target your questions towards that, target your screening exercises towards that.

Try your best to walk out of the screen with the ability to say, "They're a good fit because…" And if you fail to walk out with that, well- it's not really a statement about the candidate. It just doesn't work out. Nothing personal.

But if the code they do write during the screen is uniquely terrible, feel free to send it to us anyway. We love bad code.

[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!
Remy Porter

CodeSOD: Property Flippers

2 weeks 5 days ago

Kleyguerth was having a hard time tracking down a bug. A _hasPicked flag was "magically" toggling itself to on. It was a bug introduced in a recent commit, but the commit in question was thousands of lines, and had the helpful comment "Fixed some stuff during the tests".

In several places, the TypeScript code checks a property like so:

if (!this.checkAndPick) { // do stuff }

Now, TypeScript, being a Microsoft language, allows properties to be just, well, properties, or it allows them to be functions with getters and setters.

You see where this is going. Once upon a time was a property that just checked another, private property, and returned its value, like so:

private get checkAndPick() { return this._hasPicked; }

Sane, reasonable choice. I have questions about why a private getter exists, but I'm not here to pick nits.

As we progress, someone changed it to this:

private get checkAndPick() { return this._hasPicked || (this._hasPicked = true); }

This forces the value to true, and returns true. This always returns true. I don't like it, because using a property accessor to mutate things is bad, but at least the property name is clear- checkAndPick tells us that an item is being picked. But what's the point of the check?

Still, this version worked as people expected it to. It took our fixer to take it to the next level:

private get checkAndPick() { return this._hasPicked || !(this._hasPicked = true); }

This flips _hasPicked to true if it's not already true- but if it does, returs false. The badness of this code is rooted in the badness of the previous version, because a property should never be used this way. And while this made our fixer's tests turn green, it broke everything for everyone else.

Also: do not, do not use property accessors to mutate state. Only setters should mutate state, and even then, they should only set a field based on their input. Complicated logic does not belong in properties. Or, as this case shows, even simple logic doesn't, if that simple logic is also stupid.

[Advertisement] ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.
Remy Porter

CodeSOD: A Date with Gregory

2 weeks 6 days ago

Calendars today may be controlled by a standards body, but that's hardly an inherent fact of timekeeping. Dates and times are arbitrary and we structure them to our convenience.

If we rewind to ancient Rome, you had the role of Pontifex Maximus. This was the religious leader of Rome, and since honoring the correct feasts and festivals at the right time was part of the job, it was also the standards body which kept the calendar. It was, ostensibly, not a political position, but there was also no rule that an aspiring politician couldn't hold both that post and a political post, like consul. This was a loophole Julius Caesar ruthlessly exploited; if his political opposition wanted to have an important meeting on a given day, whoops! The signs and portents tell us that we need to have a festival and no work should be done!

There's no evidence to prove it, but Julius Caesar is exactly the kind of petty that he probably skipped Pompey's birthday every year.

Julius messed around with the calendar a fair bit for political advantage, but the final version of it was the Julian calendar and that was our core calendar for the next 1500 years or so (and in some places, still is the preferred calendar). At that point Pope Gregory came in, did a little refactoring and fixed the leap year calculations, and recalibrated the calendar to the seasons. The down side of that: he had to skip 13 days to get things back in sync.

The point of this historical digression is that there really is no point in history when dates made sense. That still doesn't excuse today's Java code, sent to us by Bernard.

GregorianCalendar gregorianCalendar = getGregorianCalendar(); XMLGregorianCalendar xmlVersion = DatatypeFactory.newInstance().newXMLGregorianCalendar(gregorianCalendar); return gregorianCalendar.equals(xmlVersion .toGregorianCalendar());

Indenting as per the original.

The GregorianCalendar is more or less what it sounds like, a calendar type in the Gregorian system, though it's worth noting that it's technically a "combined" calendar that also supports Julian dates prior to 15-OCT-1582 (with a discontinuity- it's preceeded by 04-OCT-1582). To confuse things even farther, this is a bit of fun in the Javadocs:

Prior to the institution of the Gregorian calendar, New Year's Day was March 25. To avoid confusion, this calendar always uses January 1. A manual adjustment may be made if desired for dates that are prior to the Gregorian changeover and which fall between January 1 and March 24.

"To avoid confusion." As if confusion is avoidable when crossing between two date systems.

None of that has anything to do with our code sample, it's just interesting. Let's dig into the code.

We start by fetching a GregorianCalendar object. We then construct an XMLGregorianCalendar object off of the original GregorianCalendar. Then we convert the XMLGregorianCalendar back into a GregorianCalendar and compare them. You might think that this then is a function which always returns true, but Java's got a surprise for you: converting to XMLGregorianCalendar is lossy so this function always returns false.

Bernard didn't have an explanation for why this code exists. I don't have an explanation either, besides human frailty. No matter if the original developer expected this to be true or false at any given time, why are we even doing this check? What do we hope to learn from it?

[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.
Remy Porter

CodeSOD: Contracting Space

3 weeks ago

A ticket came in marked urgent. When users were entering data in the header field, the spaces they were putting in kept getting mangled. This was in production, and had been in production for sometime.

Mike P picked up the ticket, and was able to track down the problem to a file called Strings.java. Yes, at some point, someone wrote a bunch of string helper functions and jammed them into a package. Of course, many of the functions were re-implementations of existing functions: reinvented wheels, now available in square.

For example, the trim function.

/** * @param str * @return The trimmed string, or null if the string is null or an empty string. */ public static String trim(String str) { if (str == null) { return null; } String ret = str.trim(); int len = ret.length(); char last = '\u0021'; // choose a character that will not be interpreted as whitespace char c; StringBuffer sb = new StringBuffer(); for (int i = 0; i < len; i++) { c = ret.charAt(i); if (c > '\u0020') { if (last <= '\u0020') { sb.append(' '); } sb.append(c); } last = c; } ret = sb.toString(); if ("".equals(ret)) { return null; } else { return ret; } }

Now, Mike's complaint is that this function could have been replaced with a regular expression. While that would likely be much smaller, regexes are expensive- in performance and frequently in cognitive overhead- and I actually have no objections to people scanning strings.

But let's dig into what we're doing here.

They start with a null check, which sure. Then they trim the string; never a good sign when your homemade trim method calls the built-in.

Then, they iterate across the string, copying characters into a StringBuffer. If the current character is above unicode character 20- the realm of printable characters- and if the last character was a whitespace character, we copy a whitespace character into the output, and then the printable character into the output.

What this function does is simply replace runs of whitespace with single whitespace characters.

"This string" becomes "This string"

Badly I should add. Because there are plenty of whitespace characters which appear above \u0020- like the non-breaking space (\u00A0), and many other control characters. While you might be willing to believe your users will never figure out how to type those, you can't guarantee that they'll never copy/paste them.

For me, however, this function does something far worse than being bad at removing extraneous whitespace. Because it has that check at the end- if I handed it a perfectly good string that is only whitespace, it hands me back a null.

I can see the argument- it's a bad input, so just give me back an objectively bad result. No IsNullOrEmpty check, just a simple null check. But I still hate it- turning an actual value into a null just bothers me, and seems like an easy way to cause problems.

In any case, the root problem with this bug was simply developer invented requirements: the users never wanted stray spaces to be automatically removed in the middle of the string. Trimmed yes, gutted no.

No one tried to use multiple spaces for most of the history of the application, thus no one noticed the problem. No one expected it to not work. Hence the ticket and the panic by users who didn't understand what was going on.

.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.
Remy Porter

Error'd: Pickup Sticklers

3 weeks 3 days ago

An Anonymous quality analyst and audiophile accounted "As a returning customer at napalmrecords.com I was forced to update my Billing Address. Fine. Sure. But what if my *House number* is a very big number? More than 10 "symbols"? Fortunately, 0xDEADBEEF for House number and J****** for First Name both passed validation."

And then he proved it, by screenshot:

 

Richard P. found a flubstitution failure mocking "I'm always on the lookout for new and interesting Lego sets. I definitely don't have {{product.name}} in my collection!"

 

"I guess short-named siblings aren't allowed for this security question," pointed out Mark T.

 

Finally, my favorite category of Error'd -- the security snafu. Tim R. reported this one, saying "Sainsbury/Argos in the UK doesn't want just anybody picking up the item I've ordered online and paid for, so they require not one, not two, but 3 pieces of information when I come to collect it. There's surely no way any interloper could possibly find out all 3, unless they were all sent in the same email obviously." Personally, my threat model for my grocery pickups is pretty permissive, but Tim cares.

 

[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.
Lyle Seaman

Coded Smorgasbord: High Strung

3 weeks 4 days ago

Most languages these days have some variation of "is string null or empty" as a convenience function. Certainly, C#, the language we're looking at today does. Let's look at a few example of how this can go wrong, from different developers.

We start with an example from Jason, which is useless, but not a true WTF:

/// <summary> /// Does the given string contain any characters? /// </summary> /// <param name="strToCheck">String to check</param> /// <returns> /// true - String contains some characters. /// false - String is null or empty. /// </returns> public static bool StringValid(string strToCheck) { if ((strToCheck == null) || (strToCheck == string.Empty)) return false; return true; }

Obviously, a better solution here would be to simply return the boolean expression instead of using a conditional, but equally obvious, the even better solution would be to use the built-in. But as implementations go, this doesn't completely lose the plot. It's bad, it shouldn't exist, but it's barely a WTF. How can we make this worse?

Well, Derek sends us an example line, which is scattered through the codebase.

if (Port==null || "".Equals(Port)) { /* do stuff */}

Yes, it's frequently done as a one-liner, like this, with the do stuff jammed all together. And yes, the variable is frequently different- it's likely the developer responsible saved this bit of code as a snippet so they could easily drop it in anywhere. And they dropped it in everywhere. Any place a string got touched in the code, this pattern reared its head.

I especially like the "".Equals call, which is certainly valid, but inverted from how most people would think about doing the check. It echos Python's string join function, which is invoked on the join character (and not the string being joined), which makes me wonder if that's where this developer started out?

I'll never know.

Finally, let's poke at one from Malfist. We jump over to Java for this one. Malfist saw a function called checkNull and foolishly assumed that it returned a boolean if a string was null.

public static final String checkNull(String str, String defaultStr) { if (str == null) return defaultStr ; else return str.trim() ; }

No, it's not actually a check. It's a coalesce function. Okay, misleading names aside, what is wrong with it? Well, for my money, the fact that the non-null input string gets trimmed, but the default string does not. With the bonus points that this does nothing to verify that the default string isn't null, which means this could easily still propagate null reference exceptions in unexpected places.

I've said it before, and I'll say it again: strings were a mistake. We should just abolish them. No more text, everybody, we're done.

.comment { border: none; } [Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.
Remy Porter

CodeSOD: Across the 4th Dimension

3 weeks 5 days ago

We're going to start with the code, and then talk about it. You've seen it before, you know the chorus: bad date handling:

C_DATE($1) C_STRING(7;$0) C_STRING(3;$currentMonth) C_STRING(2;$currentDay;$currentYear) C_INTEGER($month) $currentDay:=String(Day of($1)) $currentDay:=Change string("00";$currentDay;3-Length($currentDay)) $month:=Month of($1) Case of : ($month=1) $currentMonth:="JAN" : ($month=2) $currentMonth:="FEB" : ($month=3) $currentMonth:="MAR" : ($month=4) $currentMonth:="APR" : ($month=5) $currentMonth:="MAY" : ($month=6) $currentMonth:="JUN" : ($month=7) $currentMonth:="JUL" : ($month=8) $currentMonth:="AUG" : ($month=9) $currentMonth:="SEP" : ($month=10) $currentMonth:="OCT" : ($month=11) $currentMonth:="NOV" : ($month=12) $currentMonth:="DEC" End case $currentYear:=Substring(String(Year of($1));3;2) $0:=$currentDay+$currentMonth+$currentYear

At this point, most of you are asking "what the hell is that?" Well, that's Brewster's contribution to the site, and be ready to be shocked: the code you're looking at isn't the WTF in this story.

Let's rewind to 1984. Every public space was covered with a thin layer of tobacco tar. The Ground Round restaurant chain would sell children's meals based on the weight of the child and have magicians going from table to table during the meal. And nobody quite figured out exactly how relational databases were going to factor into the future, especially because in 1984, the future was on the desktop, not the big iron "server side".

Thus was born "Silver Surfer", which changed its name to "4th Dimension", or 4D. 4D was an RDBMS, an IDE, and a custom programming language. That language is what you see above. Originally, they developed on Apple hardware, and were almost published directly by Apple, but "other vendors" (like FileMaker) were concerned that Apple having a "brand" database would hurt their businesses, and pressured Apple- who at the time was very dependent on its software vendors to keep its ecosystem viable. In 1993, 4D added a server/client deployment. In 1995, it went cross platform and started working on Windows. By 1997 it supported building web applications.

All in all, 4D seems to always have been a step or two behind. It released a few years after FileMaker, which served a similar niche. It moved to Windows a few years after Access was released. It added web support a few years after tools like Cold Fusion (yes, I know) and PHP (I absolutely know) started to make building data-driven web apps more accessible. It started supporting Service Oriented Architectures in 2004, which is probably as close to "on time" as it ever got for shipping a feature based on market demands.

4D still sees infrequent releases. It supports SQL (as of 2008), and PHP (as of 2010). The company behind it still exists. It still ships, and people- like Brewster- still ship applications using it. Which brings us all the way back around to the terrible date handling code.

4D does have a "date display" function, which formats dates. But it only supports a handful of output formats, at least in the version Brewster is using. Which means if you want DD-MMM-YYYY (24-SEP-2025) you have to build it yourself.

Which is what we see above. The rare case where bad date handling isn't inherently the WTF; the absence of good date handling in the available tooling is.

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!
Remy Porter

CodeSOD: One Last ID

3 weeks 6 days ago

Chris's company has an unusual deployment. They had a MySQL database hosted on Cloud Provider A. They hired a web development company, which wanted to host their website on Cloud Provider B. Someone said, "Yeah, this makes sense," and wrote the web dev company a sizable check. They app was built, tested, and released, and everyone was happy.

Everyone was happy until the first bills came in. They expected the data load for the entire month to be in the gigabytes range, based on their userbase and expected workloads. But for some reason, the data transfer was many terabytes, blowing up their operational budget for the year in a single month.

Chris fired up a traffic monitor and saw that, yes, huge piles of data were getting shipped around with every request. Well, not every request. Every insert operation ended up retrieving a huge pile of data. A little more research was able to find the culprit:

SELECT last_insert_id() FROM some_table_name

The last_insert_id function is a useful one- it returns the last autogenerated ID in your transaction. So you can INSERT, and then check what ID was assigned to the inserted record. Great. But the way it's meant to be used is like so: SELECT last_insert_id(). Note the lack of a FROM clause.

By adding the FROM, what the developers were actually saying were "grab all rows from this table, and select the last_insert_id once for each one of them". The value of last_insert_id() just got repeated once for each row, and there were a lot of rows. Many millions. So every time a user inserted a row into most tables, the database sent back a single number, repeated millions and millions of times. Each INSERT operation caused a 30MB reply. And when you have high enough traffic, that adds up quickly.

On a technical level, it was an easy fix. On a practical one, it took six weeks to coordinate with the web dev company and their hosting setup to make the change, test the change, and deploy the change. Two of those weeks were simply spent convincing the company that yes, this was in fact happening, and yes, it was in fact their fault.

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.
Remy Porter
Checked
47 minutes 59 seconds ago
Curious Perversions in Information Technology
Subscribe to The Daily WTF feed