It's a holiday in the US, so while we're gathering with friends and family, reminiscing about old times, let's look back on the far off year of 2004, with this classic WTF. Original -- Remy

Bil Simser comments on comments ...

I'm always pleased when I see developers commenting code. It means there's something there that should be commented so the next guy will know WTF whoever wrote it was thinking. However much like any FOX special, there are times when "Comments Gone Wild". I present some production code that contains some more, err, useful comments that I've found.

// Returns: Position of the divider
// Summary: Call this method to get the position of the divider.
int GetDividerPos();

Hmmm. Glad that was cleared up.

// Summary: Call this method to refresh items in the list.
void Refresh();

Again. Good to know.

// Summary: Call this method to remove all items in the list.
void RemoveAllItems();

Whew. For a minute there I thought we would have to spend some serious debugging
time hunting down this method.

And my personal favorite...

/* this next part does something cool; don't even try to understand it*/
while(i_love_lucy_is_on_tv)
{
   xqtc_fn();
}

i_love_lucy_is_on_tv turned out to be a boolean variable set to false. Go figure.

Heh. And on an unrelated note, I've just given the good o'le two-week notice to my employer. I'll miss 'em ... they were the inspiration for starting this blog, and provided a good many of posts. I trust Jakeypoo will update us on some of their new developments, and pray I'll have nothing to share with you from my new employer.

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

https://thedailywtf.com/articles/classic-wtf-when-comments-go-wild

Lucio C inherited a large WordPress install, complete with the requisite pile of custom plugins to handle all the unique problems that the company had. Problems, of course, that weren't unique at all, and probably didn't need huge custom plugins, but clearly someone liked writing custom plugins.

One of those plugins found a need to broadcast the same method invocation across a whole pile of objects. Since this is PHP, there's no guarantee of any sort of type safety, so they engineered this solution:

function call($name, $args) {
        if (!is_array($this->objects)) return;
        foreach ($this->objects as $object) {
                if (method_exists($object, $name)) {
                        $count = count($args);
                        if ($count == 0) return $object->$name();
                        elseif ($count == 1) return $object->$name($args[0]);
                        elseif ($count == 2) return $object->$name($args[0], $args[1]);
                        elseif ($count == 3) return $object->$name($args[0], $args[1], $args[2]);
                        elseif ($count == 4) return $object->$name($args[0], $args[1], $args[2], $args[3]);
                        elseif ($count == 5) return $object->$name($args[0], $args[1], $args[2], $args[3], $args[4]);
                        elseif ($count == 6) return $object->$name($args[0], $args[1], $args[2], $args[3], $args[4], $args[5]);
                }
        }
}

I'll admit, this code itself may not be a WTF, but it points at a giant code smell of over-engineering a solution. It's also fragile code. If two underlying objects have the same name, they must take the same number of arguments, and whoever invokes this must supply than number of arguments. Oh, and nobody can accept more than 6 arguments, which should be all you ever need.

Actually, that last one probably is a good rule. Gigantic parameter lists make your code harder to read and write.

Then again, this method also makes it harder to read and write your code. It's the sort of thing that, if you ever find yourself writing it, you need to go back and re-evaluate some choices, because you probably shouldn't have. But maybe Lucio's predecessor needed to.

"Don't ever do this, until you do, but you still shouldn't have," seems to be a good description of this code.

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

https://thedailywtf.com/articles/counting-arguments

Mike's company likes to make sure their code is well documented. Every important field, enumeration, method, or class has a comment explaining what it is. You can see how much easier it makes understanding this code:

    /// 

    /// Provides clear values for Templates
    /// 
    public enum TemplateType
    {
        /// 

        /// 1
        /// 
        TEMPLATE_1 = 1,
        /// 

        /// 2
        /// 
        TEMPLATE_2 = 2,
        /// 

        /// 3
        /// 
        TEMPLATE_3 = 3,
        /// 

        /// 6
        /// 
        TEMPLATE_6 = 6,
        /// 

        /// 8
        /// 
        TEMPLATE_8 = 8,
        /// 

        /// 10
        /// 
        TEMPLATE_10 = 10,
        /// 

        /// 12
        /// 
        TEMPLATE_12 = 12,
        /// 

        /// 17
        ///        
        TEMPLATE_17 = 17,
        /// 

        /// 18
        /// 
        TEMPLATE_18 = 18,
        /// 

        /// 20
        /// 
        TEMPLATE_20 = 20,
        /// 

        /// 32
        /// 
        TEMPLATE_32 = 32,
        /// 

        /// 42
        /// 
        TEMPLATE_42 = 42,
        /// 

        /// 54
        /// 
        TEMPLATE_54 = 54,
        /// 

        /// 55
        /// 
        TEMPLATE_55 = 55,
        /// 

        /// 57
        /// 
        TEMPLATE_57 = 57,
        /// 

        /// 73
        /// 
        TEMPLATE_73 = 73,
        /// 

        /// 74
        /// 
        TEMPLATE_74 = 74,
        /// 

        /// 177
        /// 
        TEMPLATE_177 = 177,
        /// 

        /// 189
        /// 
        TEMPLATE_189 = 189
    }

There. Clear, consice, and well documented. Everything you need to know. I'm sure you have no further questions.

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

https://thedailywtf.com/articles/templated-comments

Linda found some C# code that generates random numbers. She actually found a lot of code which does that, because the same method was copy/pasted into a half dozen places. Each of those places was a View Model object, and each of those View Models contained thousands of lines of code.

There's a lot going on here, so we'll start with some highlights. First, the method signature:

// Draws a number of items randomly from a list of available items
// numToDraw: 		the number of items required
// availableList: 	the list of all possible items, including those previously drawn
// excludeList: 	items previously drawn, that we don't want drawn again
public List GetRandomNumbers(int numToDraw, List availableList, List excludeList)

We want to pull numToDraw items out of availableList, selected at random, but without including the excludeList. Seems reasonable.

Let's skip ahead, to the loop where we draw our numbers:

    while (result.Count != numToDraw)
	{
		// Add a delay, to ensure we get new random numbers - see GetRand(min, max) below
		Task.Delay(1).Wait(1);

		// Get a random object from the available list
		int index = GetRand(nextMinValue, nextMaxValue);

Wait, why is there a Wait? I mean, yes, the random number generator is seeded by the clock, but once seeded, you can pull numbers out of it as fast as you want. Perhaps we should look at GetRand, like the comment suggests.

// Returns a random value between min (inclusive) and max (exclusive)
private int GetRand(int min, int max)
{
	// Get a new Random instance, seeded by default with the tick count (milliseconds since system start)
	// Note that if this method is called in quick succession, the seed could easily be the same both times,
	// returning the same value
	Random random = new Random();
	return random.Next(min, max);
}

Ah, of course, they create a new Random object every time. They could just create one in the GetRandomNumbers method, and call random.Next, but no, they needed a wrapper function and needed to make it the most awkward and difficult thing to use. So now they have to add a millisecond delay to every loop, just to make sure that the random number generator pulls a new seed.

There are some other highlights, like this comment:

        // Copy-paste the logic above, but for the max value
		// This condition will never be true though, because nextRand(min, max) returns a number between min (inclusive)
		// and max (exclusive), so will never return max value. Which is just as well, because otherwise nextMaxValue
		// will be set to the largest value not picked so far, which would in turn never get picked.

But one really must see the whole thing to appreciate everything that the developer did here, which they helpfully thoroughly documented:

// Draws a number of items randomly from a list of available items
// numToDraw: 		the number of items required
// availableList: 	the list of all possible items, including those previously drawn
// excludeList: 	items previously drawn, that we don't want drawn again
public List GetRandomNumbers(int numToDraw, List availableList, List excludeList)
{
	if (availableList == null || numToDraw > availableList.Count)
	{
		// Can't draw the required number of objects
		return new List();
	}
	
	List result = new List();
	
	// Get limits for drawing random numbers
	int nextMinValue = 0;
	int nextMaxValue = availableList.Count;
	
	// Keep track of which items have already been drawn
	List<int> addedIndex = new List<int>();
	
	while (result.Count != numToDraw)
	{
		// Add a delay, to ensure we get new random numbers - see GetRand(min, max) below
		Task.Delay(1).Wait(1);

		// Get a random object from the available list
		int index = GetRand(nextMinValue, nextMaxValue);
		
		// Check if we have drawn this item before
		if (!addedIndex.Contains(index))
		{
			// We haven't drawn this item before, so mark it as used
			// Otherwise... we'll continue anyway
			addedIndex.Add(index);
			
			// Using a sorted list would make checking for the presence of "index" quicker
			// Too bad we aren't using a sorted list, or a HashSet
			addedIndex.Sort();
		}

		// In order to avoid redrawing random numbers previously drawn as much as possible, reduce the range
		if (nextMinValue == index)
		{
			// We have just drawn the minimum possible value, so find the next minimum value that hasn't been drawn
			int tempValue = nextMinValue + 1;
			while (true)
			{
				// Look through the list to find "tempValue". O(N), since it is a list, and I don't think the compiler
				// will know it's been sorted
				if (addedIndex.FindIndex(i => { return i == tempValue; }) < 0)
				{
					
					nextMinValue = tempValue;
					break;
				}
				if (tempValue == nextMaxValue)
				{
					// Check that we haven't drawn all possible numbers
					break;
				}
				tempValue++;
			}
		}

		// Copy-paste the logic above, but for the max value
		// This condition will never be true though, because nextRand(min, max) returns a number between min (inclusive)
		// and max (exclusive), so will never return max value. Which is just as well, because otherwise nextMaxValue
		// will be set to the largest value not picked so far, which would in turn never get picked.
		if (nextMaxValue == index)
		{
			int tempValue = nextMaxValue - 1;
			while (true)
			{
				if (addedIndex.FindIndex(i => { return i == tempValue; }) < 0)
				{
					nextMaxValue = tempValue;
					break;
				}
				if (tempValue == nextMinValue)
					break;
				tempValue--;
			}
		}

		// Check that the picked item hasn't been picked before (I thought this was the point of added index?)
		// and that it doesn't exist in the excluded list
		MyObject tempObj1 = result.Find(i => { return i.ID == availableList[index].ID; });
		MyObject tempObj2 = null;
		if (excludeList != null)
		{
			// this could have been a one-liner with the null conditional excludeList?.Find..., but that's fine
			tempObj2 = excludeList.Find(i => { return i.ID == availableList[index].ID; });

		}
		if (tempObj1 == null && tempObj2 == null)
		{
			// Hasn't been picked, and is allowed because it isn't in the exclude list
			availableList[index].IsSelected = true;
			result.Add(availableList[index]);
		}
	}
	return result;
}

// Returns a random value between min (inclusive) and max (exclusive)
private int GetRand(int min, int max)
{
	// Get a new Random instance, seeded by default with the tick count (milliseconds since system start)
	// Note that if this method is called in quick succession, the seed could easily be the same both times,
	// returning the same value
	Random random = new Random();
	return random.Next(min, max);
}

I love all the extra Sort calls, and I love this comment about them:

// Too bad we aren't using a sorted list, or a HashSet

If only, if only we could have made a different choice there.

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

https://thedailywtf.com/articles/a-sort-of-random

Jani P. relates "I ran into this appropriate CAPTCHA when filling out a lengthy, bureaucratic visa application form." (For our readers unfamiliar with the Anglo argot, "fricking" is what we call a minced oath: a substitute for a more offensive phrase. You can imagine which one - or google it.)

Cees de G. "So glad that Grafana Cloud is informing me of this apparently exceptional situation."

Wayne W. figures "there must be a difference in calculating between an iPad, where this screen capture was done, and later finalizing a somewhat lower-priced transaction on an iMac."

Jim intones

And finally, an anonymous technology consumer shares a suspiciously un-Lenovian* stock photo, writing "I hope Lenovo's software and hardware are better than their proofreading." (*correction: after some more research prompted by a commenter, I have concluded that the laptop pictured is in fact a Lenovo device, possibly an IdeaPad. It only looks like a MacBook.)

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

https://thedailywtf.com/articles/largely-middling

Alan was recently reviewing some of the scriptlets his company writes to publish their RPM installers. Some of the script quality has been… questionable in the past, so Alan wanted to do some code review.

In the uninstallation code, in the branch for AIX systems specifically, Alan found a block that needs to check that a service has successfully shut down. Since properly shutting down may take time, the check includes a pause- implemented in an unusual way.

until lssrc -s the-service-name-here | egrep 'inoperative|not'; do
  perl -e 'select(undef,undef,undef,.25)'
done

This code calls into the Perl interpreter and executes the select command, which in this context wraps the select syscall, which is intended to allow a program to wait until a filehandle is available for I/O operations. In this case, the filehandle we're looking for is undef, so the only relevant parameter here is the last one- the timeout.

So this line waits for no file handle to be available, but no more than 0.25 seconds. It's a 250ms sleep. Which, notably, the AIX sleep utility doesn't support fractional seconds- so this is potentially 750ms more efficient than taking the obvious solution.

As Alan writes:

This code is obviously worried that re-testing for service shutdown once a second with a simple "sleep 1" might risk a serious waste of the user's time. It's just so much better not to be vulnerable to a 750ms window during which the user might be distracted by browsing a cat video. Naturally this is worth the creation of a dependency on a Perl interpreter which gets invoked in order to fake a millisecond sleep timer via (ab)use of a general I/O multiplexing facility!

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

https://thedailywtf.com/articles/efficiently-waiting

As a general rule, don't invent your own file format until you have to, and even then, probably don't. But sometimes, you have to.

Tim C's company was building a format they called "generic raw format". It was solving a hard problem: they were collecting messages from a variety of organizations, in a mix of binary and plaintext, and dumping them into a flat file. Each file might contain many messages, and they needed to be able to split those messages and timestamp them correctly.

This meant that the file format needed a header at the top. It would contain information about byte order, version number, have space for arbitrary keys, and report the header length, all as text, represented as key/value pairs. Then they realized that the some of the clients and vendors supplying this data may want to include some binary data in the header, so it would also need a binary section.

All of this created some technical issues. The key one was that the header length, stored as text, could change the length of the header. This wasn't itself a deal-breaker, but other little flags created problems. If they represented byte-order as BIGENDIAN=Y, would that create confusion for their users? Would users make mistakes about what architecture they were on, or expect to use LITTLEENDIAN=Y instead?

In the end, it just made more sense to make all of the important fields binary fields. The header could still have a text section, which could contain arbitrary key/value pairs. For things like endianness, there were much simpler ways to solve the problem, like reserving 32-bits and having clients store a 1 in it. The parser could then detect whether that read as 0x00000001 or 0x10000000 and react accordingly. Having the header length be an integer and not text also meant that recording the length wouldn't impact the length.

These were all pretty reasonable things to do in a header format, and good compromises for usability and their business needs. So of course, Blaise, the CTO, objected to these changes.

"I thought we'd agreed to text!" Blaise said, when reviewing the plan for the header format.

"Well, we did," Tim explained. "But as I said, for technical reasons, it makes much more sense."

"Right, but if you do that, we can't use cat or head to review the contents of the file header."

Tim blinked. "The header has a section for binary data anyway. No one should be using cat or head to look at it."

"How else would they look at it?"

"Part of this project is to release a low-level dump tool, so they can interact with the data that way. You shouldn't just cat binary files to your terminal, weird stuff can happen."

Blaise was not convinced. "The operations people might not have the tool installed! I use cat for reading files, our file should be catable."

"But, again," Tim said, trying to be patient. "The header contains a reserved section for binary data anyway, the file content itself may be binary data, the entire idea behind what we're doing here doesn't work with, and was never meant to work with, cat."

Blaise pulled up a terminal, grabbed a sample file, and cated it. "There," he said, triumphantly, pointing at the header section where he could see key/value pairs in a sea of binary nonsense. "I can still see the header parameters. I want the file to be like that."

At this point, Tim was out of things to say. He and his team revised the spec into a much less easy to use, a much more confusing, and a much more annoying header format. The CTO got what the CTO wanted.

Surprisingly, they ended up having a hard time getting their partners to adopt the new format though…

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

https://thedailywtf.com/articles/a-binary-choice

"I work with very bad developers," writes Henry.

It's a pretty simple example of some bad code:

If Me.ddlStatus.SelectedItem.Value = 2 Then
	Dim statuscode As Integer
	statuscode = Me.ddlStatus.SelectedItem.Value
	Select Case statuscode
		Case 2
			' snip a few lines of code
			Me.In_Preparation()
		Case 3
			Me.In_Fabrication()
		Case 5
			Me.Sent()
		Case 7
			Me.Rejected()
		Case 8
			Me.Cancel()
		Case 10
			Me.Waiting_for_payment()
	End Select
Else
	Dim statuscode As Integer
	statuscode = Me.ddlStatus.SelectedItem.Value
	Select Case statuscode
		Case 2
			Me.In_Preparation()
		Case 3
			Me.In_Fabrication()
		Case 5
			Me.Sent()
		Case 7
			Me.Rejected()
		Case 8
			Me.Cancel()
		Case 10
			Me.Waiting_for_payment()
	End Select
End If

This is a special twist on the "branching logic that doesn't actually branch", because each branch of the If contains a Select/switch statement. In this case, both the If and the Select check the same field- Me.ddlStatus.SelectedItem.Value. This means the first branch doesn't need a Select at all, as it could only possibly be 2. It also means, as written, the Else branch would never be 2. In the end, that's probably good, because as the comments provided by Henry point out- there's some elided code which means the branches don't do the same thing.

This has the sense of code that just evolved without anybody understanding what it was doing or why. Lines got written and copy/pasted and re-written until it worked, the developer committed the code, and nobody thought anything more about it, if they thought anything in the first place.

Henry adds:

It's only a simple example. Most of the codebase would make your head implode. Or explode, depending on a hidden parameter in the database.

[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-select-sample

Mike fired up a local copy of his company's Java application and found out that, at least running locally, the login didn't work. Since the available documentation didn't make it clear how to set this up correctly, he plowed through the code to try and understand.

Along his way to finding out how to properly configure the system, he stumbled across its logic for ensuring that every page except the login page required a valid session.

	/**
	 * session shouldn't be checked for some pages. For example: for timeout
	 * page.. Since we're redirecting to timeout page from this filter, if we
	 * don't disable session control for it, filter will again redirect to it
	 * and this will be result with an infinite loop...
	 * @param httpServletRequest the http servlet request
	 * @return true, if is session control required for this resource
	 */
	private boolean isSessionControlRequiredForThisResource(HttpServletRequest httpServletRequest) {
		String requestPath = httpServletRequest.getRequestURI();
		boolean controlRequiredLogin = !StringUtils.contains(requestPath, "login");
		return !controlRequiredLogin ? false : true;
	}

The core of the logic here is definitely a big whiff of bad decisions. Checking if the URL contains the word "login" seems like an incredibly fragile way to disable sessions. And, as it relies on "login" never showing up in the URI in any other capacity, I suspect this could end up being a delayed action foot-gun.

But that's all hypothetical. Because TRWTF here is the stack of negations.

The logic is: If it isn't the case that the string doesn't contain "login" we return false, otherwise we return true.

As Mike writes:

It took me a good 10 minutes to assure myself that logic was correct. … This could be rewritten in a saner manner with

		String requestPath = httpServletRequest.getRequestURI();
		return !StringUtils.contains(requestPath, "login");

Anyway, It made me laugh when I saw it. My login issue at the end of the day had nothing to do with this but coming across this was a treat.

"Treat" might be the wrong word.

[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/it-s-not-what-you-didn-t-think-it-wasn-t

This week at Errr'd we return with some of our favorite samples. A screwy message or a bit of mojibake is the ordinary thing; the real gems are the errors that are themselves error dialogs. We've got a couple of those, and a few of the ordinary sort.

Stealing the worm, pseudoswede Argle Bargle comments "Generally, Disqus works well. I can even imagine the border conditions that cause my time-travel glitch. I'm even glad that the programmers planned for... for just such an emergency. Maybe it's even good programming. It's still very silly."

Insufficiently early Mark Whybird seconds "I am slightly .pst about this." Me three, Mark. It's not exactly false, it's just excruciating.

Meanwhile, Anonymous Anonymous from the Bureau of Redundancy Department, chimes "My heartfelt thanks to VS 2019 for such eloquent diagnostics."

Relentless Ruben L. squawks "I am not sure if I need to install it 7 times (or 10) to get it working right."

Finally, bulldogged Brett , getting a jump on the inevitable 2020* Christmas season supply-chain snafus, growls "I was trying to find out where my kid's present was. Guess the dialog wasn't imported either."

*Is this gag worn out yet? It is, isn't it.

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

https://thedailywtf.com/articles/any-day-now

"Retry on failure" makes a lot of sense. If you try to connect to a database, but it fails, most of the time that's a transient failure. Just try again. HTTP request failed? Try again.

Samuel inherited some code that does a task which might fail. Here's the basic flow:

bool SomeClassName::OpenFile(const CString& rsPath)
{
int count = 0;
bool bBrokenFile = false;
while(!curArchive.OpenFile(rsPath)&&!bBrokenFile)
{
        bBrokenFile = count >=10;
        if(bBrokenFile)
        {
                ASSERT(false);
                return false;
        }
        Sleep(1000);
        count++;
}

....
}

Indenting as in original.

This code tries to open a file using curArchive.OpenFile. If that fails, we'll try a few more times, before finally giving up, using the bBrokenFile flag to track the retries.

Which, we have a sort of "belt and suspenders" exit on the loop. We check !bBrokenFile at the top of the loop, but we also check it in the middle of the loop and return out of the method if bBrokenFile is true.

But that's not really the issue. Opening a file from the local filesystem is not the sort of task that's prone to transient failures. There are conditions where it may be, but none of those apply here. In fact, the main reason opening this file fails is because the archive is in an incompatible format. So this method spends 11 seconds re-attempting a task which will never succeed, instead of admitting that it just isn't working. Sometimes, you just need to know when to give up.

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

https://thedailywtf.com/articles/giving-up-too-late

Anastacio knew of a programmer at his company by reputation only- and it wasn't a good reputation. In fact, it was bad enough that when this programmer was fired, no one- even people who hadn't met them- was surprised.

The firing wasn't all good news, though. That code needed to be maintained, and someone had to do it. That's how Anastacio suddenly owned 50,000 lines of code written by his predecessor. It didn't take long to see that this didn't need to be anything like 50,000 lines long, though.

For example:

//how to remove the columns of a list with aggressive optimizations enabled
list.DeleteColumn(61);
list.DeleteColumn(60);
list.DeleteColumn(59);
list.DeleteColumn(58);
list.DeleteColumn(57);
list.DeleteColumn(56);
list.DeleteColumn(55);
list.DeleteColumn(54);
list.DeleteColumn(53);
list.DeleteColumn(52);
list.DeleteColumn(51);
list.DeleteColumn(50);
list.DeleteColumn(49);
list.DeleteColumn(48);
list.DeleteColumn(47);
list.DeleteColumn(46);
list.DeleteColumn(45);
list.DeleteColumn(44);
list.DeleteColumn(43);
list.DeleteColumn(42);
list.DeleteColumn(41);
list.DeleteColumn(40);
list.DeleteColumn(39);
list.DeleteColumn(38);
list.DeleteColumn(37);
list.DeleteColumn(36);
list.DeleteColumn(35);
list.DeleteColumn(34);
list.DeleteColumn(33);
list.DeleteColumn(32);
list.DeleteColumn(31);
list.DeleteColumn(30);
list.DeleteColumn(29);
list.DeleteColumn(28);
list.DeleteColumn(27);
list.DeleteColumn(26);
list.DeleteColumn(25);
list.DeleteColumn(24);
list.DeleteColumn(23);
list.DeleteColumn(22);
list.DeleteColumn(21);
list.DeleteColumn(20);
list.DeleteColumn(19);
list.DeleteColumn(18);
list.DeleteColumn(17);
list.DeleteColumn(16);
list.DeleteColumn(15);
list.DeleteColumn(14);
list.DeleteColumn(13);
list.DeleteColumn(12);
list.DeleteColumn(11);
list.DeleteColumn(10);
list.DeleteColumn(9);
list.DeleteColumn(8);
list.DeleteColumn(7);
list.DeleteColumn(6);
list.DeleteColumn(5);
list.DeleteColumn(4);
list.DeleteColumn(3);
list.DeleteColumn(2);
list.DeleteColumn(1);
list.DeleteColumn(0);

Anastacio is optimistic:

Given a few weeks, I will be able to shrink it to less than 10,000 [lines].

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

https://thedailywtf.com/articles/delete-column-from-list

It's time to round up a few minor WTFs today. Some are bad, some are just funny, and some make you wonder what the meaning of all of this actually is.

We'll start with Tom W. After winning a promotional contest at a fast food restaurant, he received a confirmation email. Unfortunately, at the top of that email, was the following content in plaintext:

data source=ukdev.database.windows.net;initial catalog=teamedition-Staging;
persist security info=True;user id=teamedition2021;password=acglrdu9#!%E!;
MultipleActiveResultSets=True;App=TeamEdition 
data source=ukproduction.database.windows.net;initial catalog=teamedition-production;
persist security info=True;user id=teamedition2021;password=acglrdu9#!%E
;MultipleActiveResultSets=True;App=TeamEdition GT10015020 
Scanned Code 8P46NNJ4Q8 to be told better luck next time 
Scanned code BGXTJL7TP5 Exception error 
Scanned code 9N9D43PK53 Exception error

By the time Tom received the email, the passwords had been changed.

Jaera's company has an API that is stuffed with bugs, but the team is usually responsive and fixes those quickly once they're discovered. The problem Jaera has with the API is that it's just weird. Here's an example message:

[
  {
     "id": "",
     "category": "3",
     "": "",
     "status": 200
  }
]

This just combines a bunch of mild annoyances. The HTTP status code is in the body of the JSON document. This endpoint only ever returns one object, but it's wrapped up in an array anyway.

"meowcow moocat" stumbled around this one redundant line of C#:

int LayerMask = 1 << 0;

In this one's defense, I'll actually do something similar when working with color data- rgb = (r << 16 | g << 8 | b << 0)- just to be consisent. But still, it's an odd thing to see just sorta sitting out there when setting something equal to one.

And finally, an Anonymous submitter brings us a question. You may want to trap mouse enter and mouse exit events, but what happens when your Objective-C event handling is more philosophical?

- (void)mouseExisted:(NSEvent*)event
{
    
}

This typo, of course, harmed absolutely nothing- the event handler body is empty. But it does make one think about the deep question in life, and whether or not mice exist or are just constructs of the human mind.

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

https://thedailywtf.com/articles/bad-code-exists

Over twenty years ago, Matt's employer started a project to replace a legacy system. Like a lot of legacy systems, no one actually knew exactly what it did. "Just read the code," is a wonderful sentiment, but a less practical solution when you've got hundreds of thousands of lines of code and no subject-matter experts to explain it, and no one is actually sure what the requirements of the system even are at this point.

There's a standard practice for dealing with these situations. I'm not sure it should be called a "best practice", but a standard one: run both systems at the same time, feed them the same inputs and make sure they generate the same outputs.

We cut to present day, when the legacy system is still running, and the "new" system is still getting the kinks worked out. They've been running in parallel for twenty years, and may be running in that state for much much longer.

Matt shares some C code to illustrate why that might be:

   while (i < *p_rows)
   {
      switch (she_bop.pair_number[i])
      {
         case -5:
            sell_from[j+4]=she_bop.from[i];
            sell_price[j+4]=she_bop.price[i];
            sell_bid[j+4]=she_bop.bid[i];
            break;
         case -4:
            sell_from[j+3]=she_bop.from[i];
            sell_price[j+3]=she_bop.price[i];
            sell_bid[j+3]=she_bop.bid[i];
            break;
         case -3:
            sell_from[j+2]=she_bop.from[i];
            sell_price[j+2]=she_bop.price[i];
            sell_bid[j+2]=she_bop.bid[i];
            break;
         case -2:
            sell_from[j+1]=she_bop.from[i];
            sell_price[j+1]=she_bop.price[i];
            sell_bid[j+1]=she_bop.bid[i];
            break;
         case -1:
            sell_from[j+0]=she_bop.from[i];
            sell_price[j+0]=she_bop.price[i];
            sell_bid[j+0]=she_bop.bid[i];
            break;
         case +1:
            buy_from[j+0]=she_bop.from[i];
            buy_price[j+0]=she_bop.price[i];
            buy_bid[j+0]=she_bop.bid[i];
            break;
         case +2:
            buy_from[j+1]=she_bop.from[i];
            buy_price[j+1]=she_bop.price[i];
            buy_bid[j+1]=she_bop.bid[i];
            break;
         case +3:
            buy_from[j+2]=she_bop.from[i];
            buy_price[j+2]=she_bop.price[i];
            buy_bid[j+2]=she_bop.bid[i];
            break;
         case +4:
            buy_from[j+3]=she_bop.from[i];
            buy_price[j+3]=she_bop.price[i];
            buy_bid[j+3]=she_bop.bid[i];
            break;
         case +5:
            buy_from[j+4]=she_bop.from[i];
            buy_price[j+4]=she_bop.price[i];
            buy_bid[j+4]=she_bop.bid[i];
            break;
         default:
            she_bop_debug(SHE_BOP_DBG,
                          SHE_DBG_LEVEL_3,
                          "duh");
            break;
      }
      i++;
   }

Here, we have a for-case antipattern that somehow manages to be wrong in an entirely different way than the typical for-case pattern. Here, we do the same thing regardless of the value, we just change our behavior based on a numerical offset. That offset, of course, can easily be calculated based on the she_bop.pair_number value.

That said, there are other whiffs of ugliness that we can't even see here. Why the three arrays for buy_from, buy_price, and buy_bid, when they clearly know how to use structs. Then again, do they, as she_bop seems to be a struct of arrays instead of a more typical array of structs. And just what is the relationship between i and j anyway?

And then, of course, there's the weird relationship with that pair number- why is it in a range from -5 to +5? Why do we log out a debugging message if it's not? Why is that message absolutely useless?

More code might give us more context, but I suspect it won't. I suspect there's a very good reason this project hasn't yet successfully replaced the legacy system.

[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/bop-it

https://thedailywtf.com/articles/lost-in-translation

Apolena supports an application written by contractors many years ago. It tracks user activity for reporting purposes, as one does. They then want to report on this, because why else are you gathering this information?

The contractor supplied this query to do the work.

SELECT
                REPLACE(REPLACE(REPLACE(REPLACE(REPLACE(
                    REPLACE(REPLACE(REPLACE(REPLACE(REPLACE(
                        su.social_network,'fb','Facebook'),
                        'custom','Custom Login'),'free','Free WiFi'),
                        'twitter','Twitter'),'linkedin','LinkedIn'),
                        'vkontakte','VKontakte'),'instagram','Instagram'),
                        'Twilio','SMS'),'messenger','Messenger'),
                        'password','Passcode') AS 'Login Type',
                st.return+1 as Visits,
                st.user_table_id,
                st.social_network,
                st.picture_url,
                st.email AS email,
                CONCAT(st.gender,if(st.age!='',concat(', ',st.age),'')) AS Gender,
                device,
                sum(sts.sessiontime) AS 'Session(min)',
                CONCAT(sum(sts.download),' | ', sum(sts.upload)) AS 'Down | Up (MB)'
            FROM 
                user_table AS st
            JOIN 
                user_table_sessions AS sts ON st.user_table_id = sts.stid
            LEFT JOIN 
                location AS l ON sr.location_id=l.id
            WHERE
                1=1

The REPLACE line has had whitespace added, as it was one giant line.

Which, that particular line is… one way to handle turning internal strings into display strings. It's the wrong way, but it's a way.

It's worth noting that the column names on the query are the names that are actually displayed on the report. Which makes the Gender column interesting, as if the user has a registered age, that becomes part of their gender. Imagine the doctor with the new parents: "Congratulations! It's a (boy,0)!"

As a final naming issue, Session(min) displays sum(sts.sessiontime) which is stored in seconds.

Regardless, the fix for this query is to REPLACE(REPLACE(REPLACE(REPLACE()))) it.

[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-replacement-operation

We've talked about Microsoft's WebForms in the past. Having used it extensively in the era, it was a weird mismatch, an attempt to get Visual Basic-style GUI designer tools attached to web applications, where button presses on the client side web page were exposed as events on the server side.

It also meant that you could manipulate the GUI objects on the server side, and the rendered HTML would represent that. So as part of processing a request, you might manipulate one of these "server side" controls. So, if you wanted to ensure that the search field had focus when the page loaded, you could simply invoke txtSearch.Focus() as part of handling the request.

Since you may still need to do something on the client side at page load, like maybe register run some jQuery calls to turn a text box into a fancy date picker wideget, it also offered a ScriptManager.RegisterStartupScript. This executes some JavaScript when the page loads.

If all of this sounds bad, and like it's the real WTF, you're absolutely right. But even in this terrible world, you could still make things worse.

Ino sends this:

        private void SetFocus()
        {
            ScriptManager.RegisterStartupScript(hiddenDummy, hiddenDummy.GetType(), "focus", 
            "document.getElementById('" + txtSearch.ClientID + "').focus();", true);
            ScriptManager.RegisterStartupScript(hiddenDummy, hiddenDummy.GetType(), "focus2", 
            "document.getElementById('" + txtSearch.ClientID + "').focus();", true);
            ScriptManager.RegisterStartupScript(hiddenDummy, hiddenDummy.GetType(), "focus3", 
            "document.getElementById('" + txtSearch.ClientID + "').focus();", true);
            ScriptManager.RegisterStartupScript(hiddenDummy, hiddenDummy.GetType(), "focus4", 
            "document.getElementById('" + txtSearch.ClientID + 
                "').value = document.getElementById('" + txtSearch.ClientID + "').value;", true);
            ScriptManager.RegisterStartupScript(hiddenDummy, hiddenDummy.GetType(), "focus5", 
            "document.getElementById('" + txtSearch.ClientID + 
                "').value = document.getElementById('" + txtSearch.ClientID + "').value;", true);
        }

Now, it's important to note that this is meant to run at page load. The goal is that the search box has focus after the initial load. Instead of using the server-side rendering to focus the text box, we'll use a client side script. We attach that script to hiddenDummy, which is a hidden input field in this example which exists only because you need to supply a control that "owns" the

https://thedailywtf.com/articles/unable-to-focus

There once was a developer who had a lot of hustle. They put out a shingle as a contractor, knocked on doors, made phone calls, and targeted those small businesses that needed something a little more custom than just off-the-shelf could get, but didn't have the money to afford a larger dev shop.

And after finishing a handful of projects and building a reputation, this developer took a job at a large firm, in another town, and left a lot of unhappy customers with unfinished software behind.

This is where Graeme comes in. He got a call from a local hotel who needed their booking system finished up. It had some… colorful choices.

    $sql_search = "Select * from residence_main where Active=1 ";

    if ($req_typ_id !== "NoType")
    {
        if ($req_typ_id == "1")
        {
            $sql_search = $sql_search."And type_id1=1 ";
        }
        elseif ($req_typ_id == "2")
        {
            $sql_search = $sql_search."And type_id2=1 ";
        }
        elseif ($req_typ_id == "3")
        {
            $sql_search = $sql_search."And type_id3=1 ";
        }
        // snip
        elseif ($req_typ_id == "10")
        {
            $sql_search = $sql_search."And type_id10=1 ";
        }
    }

Instead of having a single "type" column which could be mapped as essentially an enum, and maybe use a foreign key to a type table, they instead had ten type columns. Integer columns, which were used as a boolean value.

The only good thing I can see in this is that it doesn't allow for any SQL injection attacks, so that's something anyway.

    if ($req_bed_id !== "NoBed")
    {
        $sql_search = $sql_search."And Bedrooms=$req_bed_id ";
    }

    if ($req_loc_id !== "NoLoc")
    {
        $sql_search = $sql_search."And loc_id=$req_loc_id ";
    }

    if ($req_key_id !== " Keywords")
    {
        $sql_search = $sql_search."And res_desc LIKE '%".$req_key_id."%' ";
    }

Ah, there we go. I was worried for a moment that we wouldn't have a SQL injection vulnerability. Of course, even with this clear exploit, Graeme has worse news:

The query string is used without any escaping, but it would really not be necessary to bother with SQL injection. Anyone who navigated to a special super-secret URL (added the path "/mydblak" to the domain name) they would find themselves in a rather old version of PHPMyAdmin - no password or other inconvenience required.

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

https://thedailywtf.com/articles/contractor-s-leftovers

Inheritance is often described as a tool for code reuse, and those discussions inevitably wander down an alley parsing out the nature of has a and is a relationships, when to favor composition, and how inheritance trees get fragile.

When Megan D inherited a Java application which had been largely untouched since 2006, she expected it to be full of all kinds of ugly problems. But they couldn't be that ugly- the software was used and well liked by the end users. Of course, it was hard to tell if they actually liked it, or had just been with it so long they'd adjusted to its flaws, because the whole workflow was extremely fragile and frequently failed.

One feature in the large application was a reporting subsystem which could display line graphs. The three classes we'll look at form a nice inheritance hierarchy. There is the LineGraphTableModel, and then specifically the XLineGraphTableModel and the YLineGraphTableModel.

These classes exist to let the user configure the way the graph renders. The user interface displays the options that can be set for each axis in a JTable view. Each data series on the graph gets a row in the table, each setting you can control on the data series gets a column.

The first thing we'll highlight is that LineGraphTableModel contains a enum called TableColumns, which are the columns on our table.

	/**
	 * The table columns used on the SeriesSettings JTable.
	 * 
	 * @author Coop
	 */
	public static enum TableColumns {
		// DataItem, Label, Color, Points, IndAxis;
		Label, Color;

		private TableColumns() {
		}

		/**
		 * Retrieves a list of the table column names.
		 * 
		 * @return String[] The table column names.
		 */
		public String[] listColumnNames() {
			TableColumns[] aValues = TableColumns.values();
			String[] aNamesList = new String[aValues.length];
			for (int aLoopIndex = 0; aLoopIndex < aValues.length; aLoopIndex++) {
				aNamesList[aLoopIndex] = aValues[aLoopIndex].name();
			}
			return aNamesList;
		}
	}// end enum

None of this is… wrong per se. As Megan writes: "It's just that everything about it is so completely pointless, useless, over-complicated and verbose for how little it does." The method listColumnNames is a good example: while mapping enums to strings might be a useful function, this isn't actually invoked anywhere. It was included as a "just in case" thing.

That's just weird, sure, but not really a WTF.

Now the LineGraphTableModel inherits from the Java AbstractTableModel, which has a few hooks you need to implement to actually work as a table model object. The default implementation covers a lot of cases, but you may need to override a few. For example, if you want to control what column names display on the table:

	/**
	 * Implementation for AbstractTableModel to return the name of a indicated
	 * column.
	 * 
	 * @param aColIndex
	 *            The index number of the column.
	 * @return String The name of the column.
	 */
	@Override
	public String getColumnName(int aColIndex) {
		return TableColumns.values()[aColIndex].name();
	}

That's fine and pretty normal. But what about actually interacting with the values in the table? You need something for that. Let's see how that gets implemented.

	/**
	 * Implementation for AbstractTableModel to set the value in the JTable at the
	 * cell indicated.
	 * 
	 * @param aRowIndex
	 *            The row index of the cell.
	 * @param aColIndex
	 *            The column index of the cell.
	 */
	@Override
	abstract public void setValueAt(Object value, int aRow, int aCol);

Oh… that's weird. This seems like the kind of thing you'd want to implement in your base class. How different do the implementations of XLineGraphTableModel and YLineGraphTableModel need to be?

Well, let's take a diff of the two files, before we post them in their entirety:

6,7c6,8
< /**
<  * The Class XLineGraphTableModel.
---
> 
>  /**
>  * The Class YLineGraphTableModel.
9c10
< public class XLineGraphTableModel extends LineGraphTableModel {
---
> public class YLineGraphTableModel extends LineGraphTableModel {
12c13
<           log = LogFactory.getLog(XLineGraphTableModel.class.getCanonicalName());
---
>           log = LogFactory.getLog(YLineGraphTableModel.class.getCanonicalName());
26c27
<               ArrayList alSS = lgm.getXDataSeriesSettings();
---
>               ArrayList alSS = lgm.getYDataSeriesSettings();
30c31
<               }
---
>               }
36c37
<                       SeriesSettings.setXAxisColor(value);
---
>                       SeriesSettings.setYAxisColor(value);
51c52
< 
---
> 
55,57c56,58
<               ArrayList aXDataSeriesList = lgm.getXDataSeriesSettings();
<               if (aXDataSeriesList.size() < aRow) {
<                       log.debug("Invalid row: "+aRow);
---
>               ArrayList aYDataSeriesList = lgm.getYDataSeriesSettings();
>               if (aYDataSeriesList.size() < aRow) {
>                       log.debug("Invalid row: " + aRow);
60c61
<               SeriesSettings aDataSeries = aXDataSeriesList.get(aRow);
---
>               SeriesSettings aDataSeries = aYDataSeriesList.get(aRow);

They're the same implementation with the name of fields changed. And here's where we have our true WTF. These two classes are basically copy/paste versions of each other. Except they weren't exactly copy/paste versions of each other, because I cheated a smidge on the diff I posted. They didn't originally have identical whitespace- each file had a few extra line breaks in different places, implying someone was manually copy/pasting (or worse, retyping) the same code into two files.

Now, again, this is just the reporting piece of the product. There is a data parsing piece, there's also an admin piece. Megan has this to say about the entire suite of tools:

The parser is OK, I guess. It's hard-coded and brittle, but the files it reads are generated by an equally hard-coded and brittle C++ program, so it hasn't needed much attention. The admin tool is… fine, though it doesn't do much. Especially now that I took out the buttons that completely wiped out the database without so much as a warning. (Yes. Really. Three buttons that deleted tables entirely just to create them again. Pressing those three buttons in order was part of the "installation" process, but they were there after installation too - between "Test Database Connection" and "Export Processing Queue".)

In any case, here's XLineGraphTableModel in its entirety. Converting it to YLineGraphTableModel can be left as an exercise for the reader.

package com.company.project.app.TOOL_GRAPH.lineGraph;

import java.util.ArrayList;
import org.apache.commons.logging.LogFactory;

/**
 * The Class XLineGraphTableModel.
 */
public class XLineGraphTableModel extends LineGraphTableModel {
	/** The Constant log. */
	protected static final org.apache.commons.logging.Log
	    log = LogFactory.getLog(XLineGraphTableModel.class.getCanonicalName());

	/** The Constant serialVersionUID. */
	private static final long serialVersionUID = -7202378534233431981L;

	/* (non-Javadoc)
	 * @see com.company.project.app.TOOL_GRAPH.lineGraph.LineGraphTableModel#setValueAt(java.lang.Object, int, int)
	 */
	@Override
	public void setValueAt(Object value, int aRow, int aCol) {
		if (lgm == null)
		{
			return;
		}
		ArrayList alSS = lgm.getXDataSeriesSettings();
		if (alSS.size() < aRow)
		{
			return;
		}
		SeriesSettings aDataSeries = alSS.get(aRow);
		TableColumns aTableColumn = TableColumns.values()[aCol];
		switch (aTableColumn) {
		case Color:
			aDataSeries.setColor(value);
			SeriesSettings.setXAxisColor(value);
			break;
		case Label:
			aDataSeries.setLabel(value);
			break;
		}
		fireTableCellUpdated(aRow, aCol);
	}

	/* (non-Javadoc)
	 * @see com.company.project.app.TOOL_GRAPH.lineGraph.LineGraphTableModel#getValueAt(int, int)
	 */
	@Override
	public Object getValueAt(int aRow, int aCol) {
		if (lgm == null) {
			
			log.debug("Invalid lgm");
			return null;
		}
		ArrayList aXDataSeriesList = lgm.getXDataSeriesSettings();
		if (aXDataSeriesList.size() < aRow) {
			log.debug("Invalid row: "+aRow);
			return null;
		}
		SeriesSettings aDataSeries = aXDataSeriesList.get(aRow);
		TableColumns aTableColumn = TableColumns.values()[aCol];
		switch (aTableColumn) {
		case Color:
			return aDataSeries.color;
		case Label:
			return aDataSeries.label;
		default:
			return null;
		}
	}

}

[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-repetition-of-repetition-to-repeat

To atone for last week's errant Error'd, this week we're giving you 20% extra at no additional charge. For reals.

An anonymous submission starts us off. Ordinarily we wouldn't repost from social media but this is just too delicious.

Music lover Ben A. notes "There are plenty of things that accept HTML-style character references. RDS (Radio Data System) isn't one of them."

While regular contributor Mr. TA trumpets his dismay "...that tech house is now considered classical."

Shopping for a new Toyota radiowrapper Nunzio T. declares "I hope their cars don't do this!" I, for one, hope they DO. Forget Automatic Self-Driving, I want Automatic Self-Repair! Take my money!

Whereupon regular reader Peter stops to fuel up his own radiowrapper and discovers "Cool that security matters to you, but it seems the display unit does not agree with you." Commenters here will no doubt disagree with Peter (as do I).

And another anonymous radio-shopper (undoubtedly unwrapped) uncovers "So it seems that this particular mall directory was having some memory issues. Though I'm curious why a guard page would be needed in this day and age and for a directory of all things."

[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/is-this-the-real-life