Optional types are an attempt to patch the "billion dollar mistake". When you don't know if you have a value or not, you wrap it in an Optional, which ensures that there is a value (the Optional itself), thus avoiding null reference exceptions. Then you can query the Optional to see if there is a real value or not.

This is all fine and good, and can cut down on some bugs. Good implementations are loaded with convenience methods which make it easy to work on the optionals.

But then, you get code like Burgers found. Which just leaves us scratching our heads:

private static final Optional<Boolean> TRUE = Optional.of(Boolean.TRUE);
private static final Optional<Boolean> FALSE = Optional.of(Boolean.FALSE);

Look, any time you're making constants for TRUE or FALSE, something has gone wrong, and yes, I'm including pre-1999 versions of C in this. It's especially telling when you do it in a language that already has such constants, though- at its core- these lines are saying TRUE = TRUE. Yes, we're wrapping the whole thing in an Optional here, which potentially is useful, but if it is useful, something else has gone wrong.

Burgers works for a large insurance company, and writes this about the code:

I was trying to track down a certain piece of code in a Spring web API application when I noticed something curious. It looked like there was a chunk of code implementing an application-specific request filter in business logic, totally ignoring the filter functions offered by the framework itself and while it was not related to the task I was working on, I followed the filter apply call to its declaration. While I cannot supply the entire custom request filter implementation, take these two static declarations as a demonstration of how awful the rest of the class is.

Ah, of course- deep down, someone saw a perfectly functional wheel and said, "I could make one of those myself!" and these lines are representative of the result.

[Advertisement] Keep all your packages and Docker containers in one place, scan for vulnerabilities, and control who can access different feeds. ProGet installs in minutes and has a powerful free version with a lot of great features that you can upgrade when ready.Learn more.

Today's awfulness comes from Tim H, and while it's technically more than one line, it's so representative of the code, and so short that I'm going to call this a representative line. Before we get to the code, we need to talk a little history.

Tim's project is roughly three decades old. It's a C++ tool used for a variety of research projects, and this means that 90% of the people who have worked on it are PhD candidates in computer science programs. We all know the rule of CompSci PhDs and programming: they're terrible at it. It's like the old joke about the farmer who, when unable to find an engineer to build him a cow conveyer, asked a physicist. After months of work, the physicist introduced the result: "First, we assume a perfectly spherical cow in a vacuum…"

Now, this particularly function has been anonymized, but it's easy to understand what the intent was:

bool isFooOrBar() {
  return isFoo() && isBar();
}

The obvious problem here is the mismatch between the function name and the actual function behavior- it promises an or operation, but does an and, which the astute reader may note are different things.

I think this offers another problem, though. Even if the function name were correct, given the brevity of the body, I'd argue that it actually makes the code less clear. Maybe it's just me, but isFoo() && isBar() is more clear in its intent than isFooAndBar(). There's a cognitive overhead to adding more symbols that would make me reluctant to add such a function.

There may be an argument about code-reuse, but it's worth noting- this function is only ever called in one place.

This particular function is not itself, all that new. Tim writes:

This was committed as new code in 2010 (i.e., not a refactor). I'm not sure if the author changed their mind in the middle of writing the function or just forgot which buttons on the keyboard to press.

More likely, Tim, is that they initially wrote it as an "or" operation and then discovered that they were wrong and it needed to be an "and". Despite the fact that the function was only called in one place, they opted to change the body without changing the name, because they didn't want to "track down all the places it's used". Besides, isn't the point of a function to encapsulate the behavior?

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

I am on record as not particularly loving JSON as a serialization format. It's fine, and I'm certainly not going to die on any hills over it, but I think that as we stripped down the complexity of XML we threw away too much.

On the flip side, the simplicity means that it's harder to use it wrong. It's absent many footguns.

Well, one might think. But then Hootentoot ran into a problem. You see, an internal partner needed to send them a JSON document which contains a JSON document. Now, one might say, "isn't any JSON object a valid sub-document? Can't you just nest JSON inside of JSON all day? What could go wrong here?"

"value":"[{\"value\":\"1245\",\"begin_datum\":\"2025-05-19\",\"eind_datum\":null},{\"value\":\"1204\",\"begin_datum\":\"2025-05-19\",\"eind_datum\":\"2025-05-19\"}]",

This. This could go wrong. They embedded JSON inside of JSON… as a string.

Hootentoot references the hottest memes of a decade and a half ago to describe this Xzibit:

Yo dawg, i heard you like JSON, so i've put some JSON in your JSON

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!

Carlos and Claire found themselves supporting a 3rd party logistics package, called IniFreight. Like most "enterprise" software, it was expensive, unreliable, and incredibly complicated. It had also been owned by four different companies during the time Carlos had supported it, as its various owners underwent a series of acquisitions. It kept them busy, which is better than being bored.

One day, Claire asked Carlos, "In SQL, what does an exclamation point mean?"

"Like, as a negation? I don't think most SQL dialects support that."

"No, like-" and Claire showed him the query.

select * from valuation where origin_country < '!'

"IniFreight, I presume?" Carlos asked.

"Yeah. I assume this means, 'where origin country isn't blank?' But why not just check for NOT NULL?"

The why was easy to answer: origin_country had a constraint which prohibited nulls. But the input field didn't do a trim, so the field did allow whitespace only strings. The ! is the first printable, non-whitespace character in ASCII (which is what their database was using, because it was built before "support wide character sets" was a common desire).

Unfortunately, this means that my micronation, which is simply spelled with the ASCII character 0x07 will never show up in their database. You might not think you're familiar with my country, but trust me- it'll ring a bell.

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

Combining Java with lower-level bit manipulations is asking for trouble- not because the language is inadequate to the task, but because so many of the developers who work in Java are so used to working at a high level they might not quite "get" what they need to do.

Victor inherited one such project, which used bitmasks and bitwise operations a great deal, based on the network protocol it implemented. Here's how the developers responsible created their bitmasks:

private static long FFFFFFFF = Long.parseLong("FFFFFFFF", 16);

So, the first thing that's important to note, is that Java does support hex literals, so 0xFFFFFFFF is a perfectly valid literal. So we don't need to create a string and parse it. But we also don't need to make a constant simply named FFFFFFFF, which is just the old twenty = 20 constant pattern: technically you've made a constant but you haven't actually made the magic number go away.

Of course, this also isn't actually a constant, so it's entirely possible that FFFFFFFF could hold a value which isn't 0xFFFFFFFF.

Kate inherited a system where Java code generates JavaScript (by good old fashioned string concatenation) and embeds it into an output template. The Java code was written by someone who didn't fully understand Java, but JavaScript was also a language they didn't understand, and the resulting unholy mess was buggy and difficult to maintain.

Why trying to debug the JavaScript, Kate had to dig through the generated code, which led to this little representative line:

dojo.byId('html;------sites------fileadmin------en------fileadmin------index.html;;12').setAttribute('isLocked','true');

The byId function is an alias to the browser's document.getElementById function. The ID on display here is clearly generated by the Java code, resulting in an absolutely cursed ID for an element in the page. The semicolons are field separators, which means you can parse the ID to get other information about it. I have no idea what the 12 means, but it clearly means something. Then there's that long kebab-looking string. It seems like maybe some sort of hierarchy information? But maybe not, because fileadmin appears twice? Why are there so many dashes? If I got an answer to that question, would I survive it? Would I be able to navigate the world if I understood the dark secret of those dashes? Or would I have to give myself over to our Dark Lords and dedicate my life to bringing about the end of all things?

Like all good representative lines, this one hints at darker, deeper evils in the codebase- the code that generates (or parses) this ID must be especially cursed.

The only element which needs to have its isLocked attribute set to true is the developer responsible for this: they must be locked away before they harm the rest of us.

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

Sean sends us a one-line function that is a delight, if by delight you mean "horror". You'll be shocked to know it's PHP.

function proget(){foreach($_GET as $k=>$v){if($k=="h"){$GLOBALS["h"]=1;}$p=explode(",",$k);}return($p);} //function to process GET headers

Based on the comment, proget is a shorthand for process_get_parameters. Which is sort of what it does. Sort of.

Let's go through this. We iterate across our $_GET parameters using $k for the key, $v for the value, but we never reference the value so forget it exists. We're iterating across every key. The first thing we check is if a key "h" exists. We don't look at its value, we just check if it exists, and if it does, we set a global variable. And this, right here, is enough for this to be a WTF. The logic of "set a global variable based on the existence of a query parameter regardless of the value of the query parameter" is… a lot. But then, somehow, this actually gets more out there.

We explode the key on commas (explode being PHP's much cooler name for split), which implies… our keys may be lists of values? Which I feel like is an example of someone not understanding what a "key" is. But worse than that, we just do this for every key, and return the results of performing that operation on the last key. Which means that if this function is doing anything at all, it's entirely dependent on the order of the keys. Which, PHP does order the keys by the order they're added, which I take to mean that if the URL has query params like ?foo=1&h=0&a,b,c,d=wtf. Or, if we're being picky about encoding, ?foo=1&h=0&a%2Cb%2Cc%2Cd=wtf. The only good news here is that PHP handles the encoding/decoding for you, so the explode will work as expected.

This is the kind of bad code that leaves me with lots of questions, and I'm not sure I want any of the answers. How did this happen, and why are questions best left unanswered, because I think the answers might cause more harm.

If you need a unique ID, UUIDs provide a variety of options. It's worth noting that variants 1, 2, and 7 all incorporate a timestamp into the UUID. In the case of variant 7, this has the benefit of making the UUID sortable, which can be convenient in many cases (v1/v2 incorporate a MAC address which means that they're sortable if generated with the same NIC).

I bring this up because Dave inherited some code written by a "guru". Said guru was working before UUIDv7 was a standard, but also didn't have any problems that required sortable UUIDs, and thus had no real reason to use timestamp based UUIDs. They just needed some random identifier and, despite using C#, didn't use the UUID functions built in to the framework. No, they instead did this:

string uniqueID = String.Format("{0:d9}", (DateTime.UtcNow.Ticks / 10) % 1000000000);

A Tick is 100 nanoseconds. We divide that by ten, mod by a billion, and then call that our unique identifier.

This is, as you might guess, not unique. First there's the possibility of timestamp collisions: generating two of these too close together in time would collide. Second, the math is just complete nonsense. We divide Ticks by ten (converting hundreds of nanoseconds into thousands of nanoseconds), then we mod by a billion. So every thousand seconds we loop and have a risk of collision again?

Maybe, maybe, these are short-lived IDs and a thousand seconds is plenty of time. But even if that's true, none of this is a good way to do that.

I suppose the saving grace is they use UtcNow and not Now, thus avoiding situations where collisions also happen because of time zones?

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!

Jon recently started a new project. When setting up his dev environment, one of his peers told him, "You can disable verbose logging by setting DEBUG_LOG=false in your config file."

Well, when Jon did that, the verbose logging remained on. When he asked his peers, they were all surprised to see that the flag wasn't turning off debug logging. "Hunh, that used to work. Someone must have changed something…" Everyone had enough new development to do that tracking down a low priority bug fell to Jon. It didn't take long.

const DEBUG_LOG = process.env.DEBUG_LOG || true

According to the blame, the code had been like this for a year, the commit crammed with half a dozen features, was made by a developer who was no longer with the company, and the message was simply "Debugging". Presumably, this was intended to be a temporary change that accidentally got committed and no one noticed or cared.

Jon fixed it, and moved on. There was likely going to be plenty more to find.

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

Jason was investigating a bug in a bitmask. It should have been set to 0b11, but someone had set it to just plain decimal 11. The line responsible looked like this:

byte number = (byte) 11;

This code takes the decimal number 11, casts it to a byte, and stores it in a byte, leaving us with the decimal number 11.

Curious, Jason checked the blame and saw that one of their senior-most devs was responsible. Figuring this was a good opportunity to poke a little fun at the dev for a silly mistake like this, Jason sent them a message about the difficulties of telling apart decimal values and binary values when the decimal value only contained ones and zeroes.

"What are you talking about?" the dev replied back. "The (byte) operator tells the compiler that the number is in binary."

Concerned by that reply, Jason started checking the rest of the code. And sure enough, many places in the code, the senior dev had followed this convention. Many of them were wrong, and just hadn't turned into a bug yet. One of two were coincidentally setting the important bits anyway.

Now, in a vague "defense" of what the senior dev was trying to do, C doesn't have a standard way of specifying binary literals. GCC and Clang both have a non-standard extension which lets you do 0b11, but that's not standard. So I understand the instinct- "there should be an easy way to do this," even if anyone with more than a week's experience *should have known better*.

But the real moral of the story is: don't use bitmasks without also using constants. It never should have been written with literals, it should have been written as byte number = FLAG_A | FLAG_B. The #define for the flags could be integer constants, or if you're feeling spicy about it, bitshift operations: #define FLAG_A = (1 << 1). Then you don't need binary literals, and also your code is actually readable for humans.

It was difficult to track down all the places where this misguided convention for binary literals was followed, as it was hard to tell the difference between that and a legitimate cast to byte. Fortunately, there weren't that many places where bitmasks were getting set.

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

When discussing ternaries, we also have to discuss readability. While short and concise, they're in some ways too compact. But don't worry, Mark's co-worker has a wonderful simplification to ternaries. This representative line is a pattern used throughout the codebase.

pnlErrorMessage.Visible = !string.IsNullOrEmpty(errorMsg) ? true : false;

This is genius, as the ternary becomes documentation for a boolean expression, telling us when we're setting things to true or false without having to think about what the expression we're evaluating means. If there is an error message, we set the error message UI element's visibility to true. Explicit, verbose, and readable.

What we're really looking at here is the ol':

if (expression)
    return true;
else
    return false;

pattern, compressed into a single ternary. Annoying, useless, and a hint that our developer doesn't understand booleans.

[Advertisement] Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!

There's nothing inherently wrong with the ternary operator. It's just the kind of thing that gets abused.

Now, we all know how it should be used. We frequently would write something like this:

let val = arr.length>0?arr[0].id:0;

If the array contains elements, grab the first one, otherwise use a reasonable default. It's not my favorite convention, but it's fine. Nothing worth complaining about.

Lahar Shah's co-worker has a different approach to this.

// Set value for tempVariable
arr.length > 0 ? tempVariable = arr[0].id : tempVariable = null;

It's amazing how converting a ternary from an expression which evaluates to a value into a statement which changes program state makes it feel so much grosser. There's nothing technically wrong with this, but it makes me want to set the code on fire and dance naked around the flames.

This, of course, wasn't a one-off use of the ternary operator. This was how the developer used the ternary, forever and always.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!

We're all used to Java class names that are… verbose.

Arno sends us a representative Java line, which demonstrates that this doesn't end with class names.

findByCampaignStatAdvertiserIdAndCampaignStatCampaignIdAndCampaignStatStatAggTypeAndCampaignStatStatGranularity

At least the class name is just BudgetDeliveryCalculator.

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