Despite being a programmer, I make software that goes in big things, which means that my workplace frequently involves the operation of power tools. My co-workers… discourage me from using the power tools. I'm not the handiest of people, and thus watching me work is awkward, uncomfortable, and creates a sense of danger. I'm allowed to use impact drivers and soldering irons, but when it comes time to use some of the more complex power saws, people gently nudge me aside.

There are tools that are valuable, but are just dangerous in the hands of the inexperienced. And even if they don't hurt themselves, you might end up boggling at the logic which made them use the tool that way. I'm talking, of course, about pre-compiler macros.

Lucas inherited some C++ code that depends on some macros, like this one:

#define CONDITION_CHECK(condition, FailureOp) if(!(condition)){FailureOp;}

This isn't, in-and-of-itself, terrible. It could definitely help with readability, especially with the right conditions. So what does it look like in use?

switch( transaction_type )
{
    case TYPE_ONE:
    case TYPE_TWO:
        return new CTransactionHandlerClass(/*stuff*/);
    default:
        break;
}
CONDITION_CHECK( false, return NULL );

Oh.

Even if we generously wanted to permit the use of literal true/false flags as some sort of debugging flag, this clearly isn't one of the situations where that makes sense. We always want to return NULL. There's never a time where we'd flip that flag to true to not return NULL.

CSpecificTransactionClass* pTrans = dynamic_cast< CAbstractTransactionClass* >( command );
if ( pTrans )
{
    CONDITION_CHECK( pTrans, return false);
    //stuff…
}

So, we do a dynamic cast, which if it fails is going to return a null value. So we have to check to see if it succeeded, which is what our if statement is doing. Once we know it succeeded, we immediately check to see if it failed.

In this case, that CONDITION_CHECK is just useless. But why be useless when you can also be too late?

CBaseCustomer *pCustomer;
CRetailCustomer *pRetail = new CRetailCustomer;
pCustomer = pRetail;
pCustomer->SetName( pName );
CONDITION_CHECK( pRetail, return false );

So, here, we have a safety check… after we interact with the possibly-not-intitialized object. Better late than never, I suppose.

Once, at work, someone handed me a bit of lumber and a hand saw, and told me to cut it. So I started, and after about a minute of watching me fail, they pointed out that the way I'd supported the lumber was causing it to bind the saw and pinch, because I had no clue what I was doing.

Which is to say: I think the developer writing and using this macro is much like me and the handsaw. It should be simple, it should be obvious, but when you have no clue what you're doing, you might not hurt yourself, but you'll make your co-workers laugh at you.

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

https://thedailywtf.com/articles/dangerous-tools-for-dangerous-users

When moving from one programming language to another, it's easy to slip into idioms that might be appropriate in one, but are wildly out of place in another. Tammy's company has some veteran developers who spent most of their careers developing in COBOL, and now' they're developing in C#. As sufficiently determined programmers, they're finding ways to write COBOL in C#. Bad COBOL.

Methods tend towards long- one thousand lines of code isn't unusual. Longer methods exist. Every method starts with a big long pile of variable declarations. Objects are used more as namespaces than anything relating to object-oriented design. So much data is passed around as 2D arrays, because someone liked working with data like it lived in a table.

But this method comes from one specific developer, who had been with the company for 25+ years. Tammy noticed it because, well, it's short, and doesn't have much by way of giant piles of variables. It's also, in every way, wrong.

public static bool _readOnlyMode;
public static bool _hasDataChanged;

//...

public bool HasDataChanged()
{
	try {
		if (_readOnlyMode == false) {
			_hasDataChanged = true;
			
			btnPrint.Enabled = true;
			btnPrintPreview.Enabled = true;
			btnSave.Enabled = true;
			btnCancel.Enabled = true;
		}
	} catch (Exception ex) {
	}
}

So, first, we note that _reandOnlyMode and _hasDataChanged are static, reinforcing the idea that this class isn't an object, but it's a namespace for data. Except the method, HasDataChanged isn't static, which is going to be fun for everyone else trying to trace through how data and values change. Note, also, that these names use the _ prefix, a convention used to identify private variables, but these are explicitly public.

The method is marked as a bool function, but never returns a value. The name and signature imply that this is a simple "question" method: has the data changed? True or false. But that's not what this method does at all. It checks if _readOnlyMode is false, and if it is, we enable a bunch of controls and set _hasDataChanged to true. Instead of returning a value, we are just setting the global public static variable _hasDataChanged.

And the whole thing is wrapped up in an exception handler that ignores the exception. Which, either none of this code could ever throw an exception, making that pointless, or there's no guarantee that all the btn… objects actually exist and there's a chance of null reference exceptions.

Tammy has inherited this application, and her assignment is to perform "maintenance". This means she doesn't touch the code until something breaks, then she spends a few days trying to replicate the error and then pick apart the thicket of global variables and spaghetti code to understand what went wrong, then usually instead of changing the code, she provides the end user with instructions on how to enter that data in a way that won't cause the application to crash, or manually updates a database record that's causing the crash. "Maintenance" means "keep in the same way", not "fix anything".

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

https://thedailywtf.com/articles/maintaining-yourself

Sometimes, we get a submission and the submitter provides absolutely no context. Sometimes, the snippet is even really short, or the problem is "subtle" in a way that means we can't spot it when skimming the inbox.

Which is why this submission from "Bus Error" has been sitting in the inbox for seven years, and why just today, as I was skimming old submissions, I finally saw what was great about this little block of C-code.

struct sockaddr_in serv_addr;

memset(&serv_addr, '0', sizeof(serv_addr));

The goal of this block of code is to ensure that the serv_addr variable is set to all zeros before it gets used elsewhere. But instead of doing that, this sets it to all '0'- the character, not the number. So instead, this initializes all the memory to 0x30.

Now, given that the first field in a sockaddr_in is the address family, which must be AF_INET on Linux, this is definitely not going to work. Even if it was actually memsetting to 0, it still wouldn't be a correct thing to do in this case. Plus, there's a whole family of methods meant to initialize this structure to the correct usable values, so this memset shouldn't be there at all.

Without context, I suspect this was programming by incantation: someone learned the hard way that variables in C don't start with a known value, so they decided to make sure that every variable they declared got zeroed out. It was a magic spell that solved a problem they didn't understand.

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

https://thedailywtf.com/articles/mark-it-zero

One of the biggest challenges in working with financial data is sanitizing the data into a canonical form. Oh, all the numeric bits are almost always going to be accurate, but when pulling data from multiple systems, is the name "John Doe", "John Q. Doe", "J. Doe"? What do we do when one system uses their mailing address and another uses their actual street address, which might use different municipality names? Or in all the other ways that important identifying information might have different representations.

This is Andres' job. Pull in data from multiple sources, massage it into some meaningful and consistent representation, and then hand it off to analysts who do some capitalism with it. Sometimes, though, it's not the data that needs to be cleaned up- it's the code. A previous developer provided this Visual Basic for Applications method for extracting first names:

Function getFirstnames(Name)
    Dim temp As String
    Dim parts
    Dim i As Long

    parts = Split(Trim(Name), " ", , vbTextCompare)
    'For i = LBound(parts) To UBound(parts)
    For i = UBound(parts) To UBound(parts)
        temp = parts(i)
        temp = Replace(Trim(Name), temp, "")
        Exit For
    Next i
    getFirstnames = Trim(temp)
End Function

Setting aside the falsehoods programmers believe about names, this is… uh… one way to accomplish the goal.

We start by splitting the string on spaces. Then we want to loop across it… sort of.

Commented out is a line that would be a more conventional loop. Note the use of LBound because VBA and older versions of Visual Basic allow you to use any starting index, so you can't assume the lower-bound is zero. This line would effectively loop across the array, if it were active.

Instead, we loop from UBound to UBound. That guarantees a one iteration loop, which opens a thorny philosophical question: if your loop body will only ever execute once, did you really write a loop?

Regardless, we'll take parts(i), the last element in the array, and chuck it into a temp variable. And then, we'll replace that value in the original string with an empty string. Then, just to be sure our loop which never loops never loops, we Exit For.

So, instead of getting the "first names", this might be better described as "stripping the surname". Except, and I know I said we were going to set aside the falsehoods programmers believe about names, the last name in someone's name isn't always their surname. Some cultures reverse the order. Spanish tradition gives everyone two surnames, from both parents, so "Jos'e Martinez Arb'o" should be shortened to just "Jos'e", if our goal is the first name.

But there's still a more subtle bug in this, because it uses Replace. So, if the last name happens to be a substring of the other names, "Liam Alistair Li" would get turned into "am Astair", which is a potentially funny nickname, but I don't think a financial company should be on a nickname basis with their clients.

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

https://thedailywtf.com/articles/by-any-other-name

No real theme this week, just a handful of whoopses, culminating in a whoopsiedaisytopsyturvysuperduper huh?

Excited Python fan Adam B. sneaks in some low-key publicity for the 3.10 release, saying "The community is so excited about the upcoming 3.10 release, they ensured that everyone notices the announcement."

Disoriented Jon who is not from Oceania puzzles "Upon noting the two close buttons, I wondered if it would make any more sense if I actually did turn my monitor upside-down."

Inspired by a novel recipe, Michael now thirsts for knowledge. "I really want to find out more about this promotion."

While on the other hand, soupless Carl C. hungers for sustenance of the more mundane form: "Fascinating, DoorDash, but what do I have to do to get my FOOD?"

Finally, big spender Steve Y. flexes his heptakaideca-digit budget: "I guess they wanted to give me a REALLY precise figure of how much I've spent." I didn't even know you could do that with decimals!

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

https://thedailywtf.com/articles/hunger

Steffen is the tech lead for an online shop. It's the kind of online shop that sell items with large enough prices that financing is an important function. They offer loans through a payment provider, and the payment provider handles all of the details.

Anything relating to finance is complicated, and loans can get very complicated, especially these shorter-term loans. Generating a payment plan for a customer's order means creating a list of all the expected monthly installments, including the due dates, the remaining principal on the loan after the payment, how much goes to principal repayment, how much goes to interest, and the new balance of the loan after interest is added. It's a lot of information, and it's information that the payment provider can provide.

But, in the interests of keeping their application responsive when providing a potential payment plan to their end user, Steffen's company opted to implement their own financial calculations. For actual payment handling, they'd still let the payment provider handle it, but for showing the customer "if you order now, this is what your payment plan will look like!", they could do that all on their side.

Or could they? While it's complicated to calculate out all the loan elements, it's also well understood. There are libraries for it, there are copy/pasteable code snippets for it. It was complicated, but not hard.

After ten days of effort, however, it wasn't working. Their test plan was simple: since the payment provider was the source of truth, they generated 10,000 sample payment plans. They fed those plans into their system, then into the payment provider's system, and then compared the results. And the results mostly matched. Mostly.

There were a handful of test cases that were off by hundreds of Euros. That was way too much. Steffen called a meeting with the payment provider's technical staff, and developers from both companies sat down to go through the code, line by line.

It was a long, tedious meeting, and while questions were raised about the varying implementations, by the end of the meeting, both companies agreed: the code was correct. But the output was wrong. The meeting ended with nothing resolved.

So Steffen grabbed one of the developers, and they sat down to go through the test cases. They chose the one which had the largest deviation from the payment provider's values, and walked through each month of the payment plan.

On the first month, both Steffen's code and the payment provider were in perfect agreement. But on the second month, Steffen's code said it was €932.28, while the payment provider said that it was €631.54. That was more than €300 off. But on the third month, things got weirder: the payment provider's plan said that the amount of interest owed went up. Loans don't work that way: as you pay the principal, the amount of interest you pay each month goes down. "Do you think they got it wrong?" Steffen wondered.

It seemed promising, but then Steffen looked at the results in more detail. They had generated 10,000 random sample loans. This included random dates ranging from antiquity all the way out to the 30th century. It was very thorough testing.

This particular loan's first due date was October 1st, 1582. When Steffen spotted that, he laughed and turned to the developer. "This is a joke, right?" When the developer didn't see the humor, Steffen explained.

You see, Steffen didn't start life as a software developer. Before moving into the industry, he worked as a professional historian. And October 1582 was a very special month in history. A unique point in history: it was the month the Western world shifted from the Julian calendar to the Gregorian calendar.

The Julian calendar assumed a year was exactly 365.25 days, which is not correct. Following that rule adds an extra leap day roughly every century. The Gregorian calendar was more precise, and mostly accurate about such thing. But to shift calendars, the Greogrian calendar needed to be synced with the seasons. That meant removing a few centuries worth of extra days left over from the Julian calendar. Call it "paying down calendrical debt". So, October 4th, 1582 was followed by October 15th, 1582. Ten days just vanished, and October 1582 had only 21 days in it.

The payment provider's calendar system handled this perfectly well. Steffen's calendar system did not. That explained why they over-charged for October, and it also explained why it seemed like the interest payment went up- November 1582 was a normal, 30-day November.

Since they would never be originating loans in the 16th century, Steffen's team generated a new set of test data, all after the start of the 21st century. This solved the problem, and after fixing a few minor bugs, their implementation matched the payment provider's.

In the end, the Pope removed ten days from October 1582, and Steffen's team put them back, spending almost exactly that time on fixing this bug.

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

https://thedailywtf.com/articles/ten-days

"When handling user input, make sure to handle exceptions," is good advice. But sometimes, that can lead you astray.

Bob is a junior programmer surrounded by veterans with 20+ years of experience. Normally, this means that when there's something unusual in the code, Bob writes it off as the senior devs vast experience. But not this time.

    Private Function IsANumber(ByVal theNumber As String) As Boolean
        Try
            If IsNumeric(theNumber) Then
                Return True
            Else
                Return False
            End If
        Catch ex As Exception
            err.Text = "There was an error." & ControlChars.CrLf & "Please try again later."
            err.CssClass = "cssLblError"
            Dim log As New logFile(Session("ConnectionString"), ex, "")
        End Try
    End Function

At first glance, this just looks like some pretty basic silliness. First, this really exists to be a wrapper around IsNumeric, a built-in function. Instead of the If/Else we could just Return IsNumeric(theNumber). Also, IsNumeric never throws an exception, making the exception handling entirely unnecessary.

But rereading it, there's more lurking here, via implication. This is an ASP.Net WebForms application. err is clearly a web control on that form, and here we write the error message directly to the control. And this suggests some broader problems.

First, that error message is terrible. It conveys no information at all. But worse, because we handle the exception right here and don't raise it up, we can assume execution will continue. Now again, in this case, we'd never hit the exception in the first place, but if we assume this is their exception-handling pattern, it strongly implies that no matter how many errors happen in handling user input, you only get one error message which tells you nothing about what actually went wrong. And if it's usually something like "try again later", that tells the user that their input was fine, and there's just something wrong in your application.

Finally, we create a class called logFile, which takes a ConnectionString as its input, which implies it's not logging to a file, but logging to a database, which just to make sure things get extra confusing.

So it's not that this code is some particular WTF horror. It's that this code is representative of these long-time software development veterans' efforts. This may just be a code smell, on its own, but there's a stench someplace in the codebase.

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

https://thedailywtf.com/articles/exceptional-numbers

Yesterday, among many many problems in that gigantic switch statement, we highlighted one line that was a real mess of mixing Java's primitive/object types.

(new Integer(1).equals(job.getSource().getLength()))

Well, Daniel has an even worse example, but this one comes with an origin story.

Many, many years ago, in the early 2000s, Daniel worked for a company that made banking software. As this was the early 2000s, their flagship product remained a text-based UI accessed via a terminal emulator (or, in some rare cases, even dumb terminals). Obviously, continuing this TUI approach wasn't going to work in the future, so the company needed to pivot to a Java-based web application.

The CEO set the direction: "We need a modern, web-based interface to sell banks. How much would it cost to implement that?" Well, Daniel's team put together the estimate, which contained a large number of trailing zeroes before the decimal point. Too many, from the CEO's perspective, so the CEO went shopping for a third-party developer who could deliver the software on the cheap.

Again, this was the early 2000s, which meant the CEO was filtering through a dozen different obscure offshoring companies. The CEO wasn't impressed by most of them, but one offshore company came to the meeting with a wireframe of the application built in PowerPoint. The CEO was impressed that they already had a UI where you could click buttons and navigate to the next screen, and took this to mean that they were basically done already. He wrote them a check for an amount in the millions, and off they went to work.

Many months later, the offshore team handed off the code. The first time, it didn't build. The second time, what unit tests there were didn't pass. The third time, it didn't build again. By the fifth time, the offshore team handed them Java code which would build and run, and only crash unexpectedly some of the time.

This code used certain… patterns. The same basic approaches to doing simple tasks were reused everywhere, because clearly this code base was implemented by copy-pasting snippets from other projects, from elsewhere in this project or from the first hit on a search result.

On such pattern was how they handled passing primitive types around:

int x = new Integer( 0 ).intValue();
someMethod( new Integer( x ) );

Talk about thinking out of the box. "Boxing", of course, being the name for the operation where you convert a primitive type to an object type, something Java can do mostly transparently in the first place. And while misusing boxing is a frequent Java anti-pattern, this one really doubles down on it.

We box the literal 0 as an object, then get its primitive value to store it as a variable, then we box the primitive variable as an object again. And someMethod was almost always defined in terms of a primitive type, so Java automatically unboxes that object back into a primitive.

Daniel adds:

And these automaton programmers copy/pasted this little piece of gold all over the system… it was nice identifying it and correcting it, but then of course, they'd blame us whenever something went wrong because we touched their original code.

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

https://thedailywtf.com/articles/inside-a-box

If you have an object, say a Job, which can be in multiple states, you'll frequently need to execute different code, depending on the state. Enter the ever useful switch statement, and all those problems are solved.

Or are they? Reva found this Java code. Enjoy your 112 line switch statement:

	@Override
	public void processEvent(Job job, Error reason) {
		switch (job.getState()) {
			case NEW:
			case WAITING:
			case STARTED:
			case ERROR:
				String message = null;
				switch (reason) {
					case ERROR:
                        // release job (no break)
					case ERROR_OTHER_1:
						// set message to some value
					case ERROR_OTHER_2:
						SomeEntity someEntity = service.get(job.getSomeEntityId());
						if (!someEntity.getStateReference().getLocationReference().getLocationTypeReference()
								.getName().equals(Constants.SOME_CONSTATNT)) {
							// Set Job and then break
							break;
						}
						// set message (no break)
					case ERROR_OTHER_3:
						// set message (no break)
					case ERROR_OTHER_4:
						// set message (no break)
					case ERROR_OTHER_5:
						// set message inline if (no break)
					case ERROR_OTHER_6:
						if (null != message) {
							// Change state of someEntity
						}
						
						if (null != job.someReference()) {
							// set job to another reference
						} else {
							//cancel job
						
							if (null != job.getSuccessor() && null != job.getSuccessor().someReference()) {
								SomeEntity otherSomeEntity = service.get(job.getSomeEntityId());
								if (!otherSomeEntity.getStateReference().getLocationReference().getLocationTypeReference().getName()
										.equals(Constants.SOME_OTHER_CONSTANT)) {
									if (OtherConstants.BLOCKED.equals(job.getSuccessor().getState())) {
										// set state of successor reference
									}
								} else {
									job.setSuccessor(recursiveSetNewLoadOrCancel(job.getSuccessor(), reason));
								}
							}
							// set job to something else
							if (new Integer(1).equals(job.getSource().getLength())) {
								// set temp String
								try {
									// try something with tmp String
								} catch (Exception e) {
									log.warn("Exception", e);
								}
							}
							// Here an application event is fired
							try {
								load = someService.get(job.getSomeEntityId());
							} catch (Exception e) {
								log.warn(e, e);
							}
							
                            // remove some messages

							if (load != null) {
								String text = // some text describing error;
								log.warn(text);
							}
						}
						break;
					case ERROR_OTHER_7:
						// save error
						break;
					case ERROR_OTHER_8:
						// set job state
						if (null != job.getSuccessor() && null != job.getSuccessor().getRequestPosition()) {
							
							if (// more checks) {
								if (// more checks) {
									// set job state to another state
								}
							} else {
								job.setSuccessor(recursiveSetNewUnitLoadOrCancel(job.getSuccessor(), reason));
							}
						}
						// get another job reference
						if (new Integer(1).equals(job.getSource().getLength())) {
							// get a string based on job
							try {
								// remove a message and save
							} catch (Exception e) {
								log.warn("Exception", e);
							}
						}
						// Fire another event
						// save an error message
						break;
					default:
						// nothing else
						log.warn(String.format(" %s no behavior defined for %s", reason, job));
				}

				break;
			case BLOCKED:
			default:
				// nothing else
		}
	}

Now, Reva has stripped out a lot of the details, but we can definitely see the core flow, with the main notable feature being an absence of breaks. A lot of missing break statements. This means that the states of NEW, WAITING, and STARTED all go down the error handling path, though presumably reason won't have a value in those cases, so it just dumps to printing a warning in the log.

We have a switch in a switch, for handling errors, which is always a treat. And even within those switches, we have loads of conditional statements. Tracing through this code is nigh impossible.

If I had to pick a favorite line in this knot of code, though, it would have to be this one:

(new Integer(1).equals(job.getSource().getLength()))

It's not wrong, but nobody writes code this way. There's no reason to do this this way. It's just the most obtuse way to express this idea, and I assume that's intentional obfuscation.

And obfuscation is what Reva suspects was the point:

I don't know how. I don't know why. It seems a previous co-worker wanted to make himself in-expendable.

With any luck, that co-worked has been expended.

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

https://thedailywtf.com/articles/switching-jobs

Just because someone writes in C, and not some more modern language doesn't mean they're going to be an expert in how to operate on binary values.

Carl W found this bit of C code with a simple goal: count the number of trailing zeroes in a 32-bit integer. At least, that's what the function name tells us. Reading the logic, I'm less certain.

u32_t FCN_count_trailing_zeros(u32_t x)
{
   u32_t c = 32u; // c will be the number of zero bits on the right
   if (x != 0u) // if n == 0, skip and return c = 32
   {
      x &= (unsigned)-(signed)x;
      c = 31u;
      if ((x & 0x0000FFFFu) != 0u) 
      { 
         c = 15u;
      }
      if ((x & 0x00FF00FFu) != 0u) 
      { 
         c -= 8u;
      }
      if ((x & 0x0F0F0F0Fu) != 0u) 
      { 
         c -= 4u;
      }
      if ((x & 0x33333333u) != 0u) 
      { 
         c -= 2u;
      }
      if ((x & 0x55555555u) != 0u) 
      { 
         c -= 1u;
      }
   }
   return c;
}

I'm a simple person, so I'd be tempted to just do this with a loop and a bit-shift operation. Now, it's probably wrong to say that this programmer doesn't know how to operate on binary values. I think they've got a better grasp on it than most of us, because it's hard to understand what's going on here but this code works, at least after a few quick tests.

We start by assuming that, at most, a 32-bit integer can contain 32 zeros, by setting c = 32u. That seems pretty reasonable, at first glance.

Some of it just looks bizarre at first glance, if you aren't used to some of the more unique C idioms: x &= (unsigned)-(signed)x;

This converts x to a signed value, negates it, then converts it back to an unsigned value. In C, this involves adding the maximum int plus one to the negative value until it's positive. When then and this with the original value. Now, assuming max-int is 0xFFFFFFFF, this has an interesting effect: it strips numbers to their first significant bit. 256 stays 256, but 257 becomes 1- because the first bit has to be set.

Great, now that we've done that, we assume at most 31u leading zeros (we've discarded zero as a possibility at this point in that conditional). Then we do a series of & operations with bitmasks. If the result isn't zero, this tells us something about the number's internal structure, which we can use to count the number of leading zeros.

I'll leave it as an exercise to the reader to trace through how, but this does work in some cursory testing. Which leaves us with why? Why do it this way?

I think it's an attempt at optimizing. This doesn't seem like code written for humans, it seems like code written for the compiler. Code that leverages branches and an understanding of how the resulting assembly gets executed to squeeze out an extra few CPU cycles.

There's just one problem: this isn't performance critical code. And so we hit upon the real WTF: premature optimization.

Actually, no, the real WTF is the FCN_ hungarian notation. I tried real hard, but I couldn't let that slide.

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!

https://thedailywtf.com/articles/wearing-a-mask

Repentinus submitted a pull request to a project he was working on. The Python code Repentinus submitted was this:

if unit.is_translated() and (self.include_fuzzy or not unit.is_fuzzy()):
    return unit.target
else:
    return unit.source

Now, this may not be the absolute most readable option here, but it's pretty clear, even without knowing the domain. We return the "target" if our object is_translated, but only if it is either "not fuzzy" or we've checked the box to "include fuzzy" options. Otherwise, we return the source. It's a fine solution, rooted in the problem domain. There's room to quibble over how the condition is expressed, but it makes sense.

Well, it doesn't make sense to one of Repentinus's collaborators. They added a commit to the pull request with the comment "Let's make that logic readable by mere mortals".

Now, a moment ago, I said Repentinus's patch was fine, and it was certainly readable enough, but apparently I was wrong. Because this is what the collaborator turned it into, the version for "mere mortals":

if unit.is_translated():
  return unit.target
if self.include_fuzzy and unit.is_fuzzy():
  return unit.target
return unit.source

This, as you can see, is wrong. It doesn't have the the same flow at all, as the goal of the conditional in the first place was to return the translated version, *but if it was "fuzzy", we only returned it when the include_fuzzy flag was set.

The collaborator refused to budge on this change. They were absolutely convinced that they had streamlined the logic, and Repentinus was just having a "snit" because the commit comment wasn't "polite" enough. Of course, the change didn't pass the unit tests, so after an overlong comment chain, the collaborator deleted the commit and pretended like they'd never done it.

To borrow a quote: "Lord, what fools these mortals be!"

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

https://thedailywtf.com/articles/for-mere-mortals

Today, let's just start with a single line of Java code, from Rich.

T oldValue = (T) (configItems.get(configKey) == null ? null : configItems.get(configKey));

This is a pretty standard bad ternary. If a value is null, we set a variable equal to null, if it's non-null, we set a variable to its value. So this whole thing really does nothing, and could just be a call to configItems.get.

Except it's actually worse. This application is multi-threaded, and configItems is a standard Java HashMap, which is not thread safe. This creates all sorts of fun race conditions, and in this specific case, it means that oldValue could be set to null when the value at configKey is actually set, which triggered a bunch of downstream bugs in actual practice.

Except that it's actually worse. Because this line of code wasn't always like this. It used to look like this:

T oldValue = (T) (configItems.get(configKey) != null ? configItems.get(configKey) : null);

This is the initial version of the line, which as you can see, just reverses the condition. It's otherwise the same line. This line was sitting in production code for a big chunk of the product's lifetime, until one day when the team added Checkstyle and PMD rules to their project. Those linting rules didn't like the original version of this line, so one of Rich's peers "fixed" the line by futzing with it until the linter passed it.

So the initial line was a mistake. Linting rules, quite helpfully, drew attention to that mistake. A programmer looked at that line, and their best approach to fixing that mistake was just to reverse it.

Rich sums up: "I guess this is proof that linting rules only get you so far. Remember, only YOU can prevent terrible code!"

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

https://thedailywtf.com/articles/retern-your-linter

Some languages support operator overloading. Others, like Java, do not. There are good arguments on either side for whether or not you should support the feature, but when you don't, you need an easy way to get the effects of operators without the syntactic sugar.

For example, if you have a custom class, and you need a way to perform comparisons on them, you may choose to implement the Comparator interface. This allows you to add some kind of comparison to an object without actually touching the object's implementation. For example, you might do something like:

class TransactionDateComparer implement Comparator {
    @Override
    public int compare(Transaction t1, Transaction t2) {
        return t1.transDate.compareTo(t2.transDate);
    }
}

You can now pass objects of this type off to SortedSets, Arrays.sort, or any other situation where you want to order transactions. And, since Comparator is a functional interface, you can even just use a lambda instead, e.g.:

Arrays.sort(transactions, (Transaction t1, Transaction t2) -> t1.compareTo(t2));

There: you now know more about generics, interfaces, lambdas, and Java than Dave's co-worker. Because Dave's co-worker understood the need for comparators, but decided that implementing a new comparator for every new type that needs to be sorted was just too much work. So they did this instead:

public class GenericComparator implements Comparator<Object> {

    @Override
    public int compare(Object o1, Object o2) {
        if (o1 instanceof A) {
            …
        } else if (o1 instanceof B) {
            …
        } else if (o1 instanceof C) {
            …
        } else if (o1 instanceof D) {
            …
        } …
    }
}

It's a good rule of thumb that if you're using an operator like instanceof, you've made a mistake somewhere. Like any rule, there are exceptions, but this is absolutely not one of them. They've abused generics to make a GenericComparator which generically only works on explicitly named classes. It's a mega-method, with one if branch for every domain class in their application. And it's used everywhere.

When Dave pointed out that this probably wasn't a good solution to their problem, the developer responsible countered that any time they needed to sort, they always knew what comparator type to use. "All the logic is encapsulated, which is very object oriented."

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

https://thedailywtf.com/articles/a-fairly-generic-comparison-to-make

Dating from a few months back, our opening submission for the week is only funny if you don't see it as a painful forecast of some coming summer in the City of Lights. Keeping with the theme of trapped systems, mostly Windows, here follow more multinational kiosk misadventures.

"Wear sunscreen," warns Thomas, temperately. "It's unseasonably warm."

Swiss M'at'e still sees the sunny side: "Sure, you can't see where your bus is going to, but at least they are providing you with the means to fix the problem..." [Teamviewer, that is, the remote-tech-support tool]

Kiwi Peter G. exclaims "Argh, got on the wrong train! I was supposed to meet friends for dinner in E3F12C, not E3F12B." The real woe is that E3F12C is a strictly Vegemite ville, but E3F12Bers are Marmite mavens. I hope he didn't go hungry.

Elswhere in the Antipodes, tourist Peter "Visited the Royal Mint in Canberra where all the information screens were displaying this message."

Finally, phoning it in from the Low Countries, Mark shares this Dutch treat. "Windows Update needs to restart, but Windows update is preventing a restart."

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

https://thedailywtf.com/articles/paris-in-the-the-spring

One of the advantages of interpreted languages, like Python, is that when your vendor ships a tool that uses Python… you can see how the Python bits actually work. Or maybe seeing how the sausage is made is a disadvantage?

Delana's vendor tool needs to determine the IP address of the computer in a platform independent way. The "standard" way of doing this in Python is to check the computer's hostname then feed that into a function like one of these which turns it into IP addresses.

Those methods should behave more-or-less the same on both Windows and Linux, so checking the IP address should be a simple and short function. Let's see how Delana's vendor did it:

def get_ip_address(ifname):
    os_type = 'windows'
    if sys.platform == 'linux' or sys.platform == 'linux2':
        os_type = 'linux'
    else:
        if sys.platform == 'win32':
            os_type = 'windows'
    if os_type == 'linux':
        import fcntl
        s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
        f = fcntl.ioctl(s.fileno(), 35093, struct.pack('256s', ifname[:15]))[20:24]
        ip = socket.inet_ntoa(f)
        return ip
    if os_type == 'windows':
        ip = [(s.connect(('8.8.8.8', 80)), s.getsockname()[0], s.close()) for s in [
         socket.socket(socket.AF_INET, socket.SOCK_DGRAM)]][0][1]
        return ip

Let's start with the OS check. We set os_type = 'windows', then check the platform. If it's 'linux' or 'linux2', our os_type is 'linux'. Otherwise, if the platform is 'win32', we set os_type to 'windows', which is what it already was. The entire 'windows' branch of the if isn't needed. Also, the "proper" way of writing it would be elif.

Okay, that's silly, but fine. What if we want to check the IP on Linux? Well, this solution pulls in the fcntl library, which is a Python wrapper around the fnctl.h collection of syscalls. They open a socket, get the filehandle, and then do a cryptic ioctl operation which I'm sure I'd understand if I pored over the manpages for a bit. A quick call to inet_ntoa turns the integer representing the IP back into a string representing the IP.

From this, I get the sense that the original developer is a C programmer who's doing Python against their will. And more to the point, a Linux C programmer that has an understanding of low-level Linux syscalls. Which is also supported by their approach to this question on Windows.

Before we talk about the Python code, let's just talk about their process. They open a socket to one of Google's DNS servers, then calls getsockname() to get the IP address. Now, they're not the only ones to use this basic approach. Since this is a UDP socket that they're opening, they're not actually starting a connection, nothing actually gets sent to Google's DNS server (and they could use any IP, since we don't need a valid route). It's weird, but not wrong.

But then there's the actual line, which is such an utter mess of a way of writing this I need to break it into bits to explain the abuse of Python syntax constructs.

Let's start with this bit, as this is easy:

socket.socket(socket.AF_INET, socket.SOCK_DGRAM)

That just creates the socket. But we wrap it up in a generator expression:

for s in [
         socket.socket(socket.AF_INET, socket.SOCK_DGRAM)]

We turn that socket into a list of one item, and then say that we're going to iterate across that, to generate a list of tuples:

[(s.connect(('8.8.8.8', 80)), s.getsockname()[0], s.close()) for s in [
         socket.socket(socket.AF_INET, socket.SOCK_DGRAM)]][0][1]

And this is where we're really going into a special place with this code. We have three operations we want to perform. We want to open the socket, check the IP address, and then close the socket. This code does that inside of a tuple: the first item in the tuple is the connect, then we getsockname, then we close.

Finally, we grab the 0th element of the list (the tuple itself), and then the 1st element in the tuple- the result of s.getsockname()[0].

The "turn the socket into a list and iterate across that list to generate a new list of one tuple" seems like an attempt to code-golf this down into one line. Maybe they were so angry about having to do a Windows version that they didn't want to spend any more lines of code on it than they had to?

I don't know, but I have a bad feeling this isn't the last time I'm going to encounter someone performing a sequence of operations in a tuple like this.

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

https://thedailywtf.com/articles/a-proper-mode-of-address

Kaci had one of "those" co-workers who didn't believe in source control. As you modified code, you'd just comment out bits and pieces, maybe replace some of it. That way you could see how the code evolved without having to learn how to use git log correctly.

At least, that was the rough argument. But it meant that when Kaci wanted to understand why this block of PHP code existed, she needed to ask the developer:

//$_GET['debug'] = TRUE;
//exit;
if (rand(1,20) != 4) {
  //echo 'im out!';
  exit;
}

So this code generates a random number between one and twenty, and if it isn't four, exits. Why? Well, the offending developer had an explanation.

"This PHP script runs as part of a cronjob. Instead of editing the crontab to change the schedule, I wanted to be able to change the schedule in code. This is how I ensure it only runs with a specific frequency."

This left Kaci burdened with even more questions than when she started. But at least now she understood why her client-side requests for data served up by that PHP were failing almost all the time.

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

https://thedailywtf.com/articles/a-graceful-exit

Sometimes, readers submit code and say, "every line is wrong!" Usually, they mean that every line contains a bad choice, or represents a bad way of accomplishing the program's goal, or there's a series of compounding mistakes.

David sends us a block which… well, yes, every line is wrong, because every line contradicts every other one.

    /**
    * gets instance of Product Status object
    *
    * @return object Warehouse Supplier object
    */    
    function __construct() {
      $this->_price = new OrderValidator();
    }

The only thing "correct" about this is that PHP does have you name your constructor __construct. Ignoring that, between the comment and the one line of executable code, not a single thing agrees with anything else. I'd suspect that this was progamming by copy/paste, but I feel like that'd be more coherent. It might be really bad AI generated code, but David assures us that this was written by an actual human.

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!

https://thedailywtf.com/articles/not-to-contradict-you

I have a very casual interest in magic tricks. I certainly don't have the patience to actually learn to perform any well, but I enjoy watching them and piecing together how they're done. What really captures my interest is that actually performing a trick is about following a well-defined path, and by following that path you can achieve incredibly impressive effects. But if you just see the effect without understanding the path, and try and replicate it yourself, it's probably not going to work.

In programming, there are certain things I consider "frickin' magic". They can create impressive effects, but the actual mechanism is concealed behind enough layers of abstraction that, if you don't stick to the carefully defined path, you can make some mistakes.

Object-Relational Mappers and .NET's LINQ features leap to mind as peak examples of this. We have a lot of LINQ related WTFs on the site.

Today's is a simple one, which highlights how, when one attempts to replicate the magic without understanding it, problems arise.

var vendor = connection.Query(get, new { id = id });
var vendorlist = vendor.ToList();
vendor.First().Contacts = this.GetContactById(vendorlist[0].Id);
vendor.First().Products = this.GetVendorProducts(id);
return vendor.Any() ? vendor.First() : null;

Here, we're using Dapper as our ORM. The goal here is to get one vendor, based on ID. Since we know ID is unique, since we know this should only ever return one value, at most, there's a method already extant for doing that. QueryFirstOrDefault will run a query, returning either the first item in the result set or null.

But we don't do that. We just fetch a result set that will, at most, contain one item, and then convert it into a list.

Which brings us to this line:

vendor.First().Contacts = this.GetContactById(vendorlist[0].Id);

What bothers me about this is that we have two totally different ways of fetching the first item in a list- by calling .First() or by indexing [0]. I don't really care which one you do, but please be consistent, at least for the duration of a single line.

Also, it's almost certainly true that GetContactById and GetVendorProducts are running additional queries, because while you can definitely use your ORM to fetch related entities all in one query and thus improve performance, you know this code isn't doing that.

But all this brings us to the cherry on top, because this line, right here, highlights a lot of misunderstandings about the magic:

return vendor.Any() ? vendor.First() : null;

.Any() returns true if there are any elements. If there are, we can return vendor.First(), because if there aren't calling vendor.First() would raise an exception. So it's good that we do a null check, but it's a pity we didn't do it before we did the previous two lines.

"Better late than never" isn't that much better.

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!

https://thedailywtf.com/articles/magical-thinking

Frequently we are sent submissions harking from that rainforest retailer, or the other one, or one of the smaller ones. It doesn't matter which, the wtferry is limited as these are almost always just some hack supplier uploading bogus data. TRR only cares about speedy delivery, and TOO cares about being cheapest, while OOTSO are just trying to keep their bricksnmortar shops alive. I don't usually run these, but this week I've got a bunch of them saved up so I'm giving you six for the price of NaN.

First up from an anonymous puppet maestro is this industrial glover on fire sale. You'd be fool not to take two! "But the shipping is the real bargain," says the maestro.

Our parsimonious (and presumably Pole) pal Patryk bargains "I think I will pass on the offers altogether."

While farther west, spendthrift Gerald splashes "I can activate these discounts on my loyalty card!" Just kidding. Gerald intends to follow Patryk's plan.

Effusive Andy proves there's something about the mobile trade that really draws the deep discounts. "If you've ordered Samsung's new eye-wateringly expensive phone, you won't want to miss these offers on accessories!" he gushed.

Gentleman George demurs "my scalp is WAY too sensitive for that." <>

But if George lights for the jungle, he might find himself found out by David L's Amazon find. "Nice to see there's no sexism in the world of trail cameras, although it's no good for trying to snag a picture of any baby animals." Leave my monkey alone, David!

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!

https://thedailywtf.com/articles/deal-of-a-lifetime

Lets say you have a 64-bit integer. You want the first 42-bits. Now, if your language has a bitshift operator, you'd be tempted to do something like largeNumber >> 22.

But what if your language also has all sorts of advanced stream processing and map functions? It'd be a shame to not use those. Tarah's shop uses Rust, and thus they do have all those exciting advanced stream processing. Her co-worker decided they just needed to be used:

let binary = format!("{:b}", snowflake);
let in_binary: u64 = binary[..42]
    .to_string()
    .chars()
    .rev()
    .enumerate()
    .map(|(idx, digit)| {
        (u64::from_str_radix(&digit.to_string(), 2).unwrap()) * ((2_u64).pow(idx as u32))
    })
    .sum();

This starts by taking a number, and converting it into a string containing the binary representation. Then we chop off the first 42 characters of that string, break them into characters, reverse the stream, enumerate it, and then apply a map where we parse each character back into a number (which will be 0 or 1), and then multiply it by two raised to the power of the index we're on. sum all that up, and we've got the first 42 bits of a 64-bit number.

And sure, snowflake >> 22 would be shorter, clearer, and far more efficient, but it wouldn't let you show off all the cool language features you have at your disposal, and isn't that what's really important?

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!

https://thedailywtf.com/articles/a-bit-overcomplicated