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.