Spaced Out Prefix – The Daily WTF

Alex had the misfortune to work on the kind of application which has forms with gigantic piles of fields, stuffed haphazardly into objects. A single form could easily have fifty or sixty fields for the user to interact with.

That leads to C# code like this:

 private static String getPrefix(AV_Suchfilter filter)
{
	String pr = String.Empty;
	try
	{
		int maxLength = 0;
		if (filter.Angebots_id != null) { maxLength = getmaxLength(maxLength, AV_MessagesTexte.Reportliste_sf_angebotsID.Length); }
		if (filter.InternesKennzeichen != null) { if (filter.InternesKennzeichen.Trim() != String.Empty) { maxLength = getmaxLength(maxLength, AV_MessagesTexte.Reportliste_sf_internesKennzeichen.Length); } }
		if (filter.Angebotsverantwortlicher_guid != null) { maxLength = getmaxLength(maxLength, AV_MessagesTexte.Reportliste_sf_angebotsverantwortlicher.Length); }

		
		

		int counter = 0;
		while (counter " ";
			counter++;
		}
	}
	catch (Exception error)
	{
		ErrorForm frm = new ErrorForm(error);
		frm.ShowDialog();
	}
	return pr;
}

The “Do this another 50 times” is doing a lot of heavy lifting in here. What really infuriates me about it, though, which we can see here, is that not all of the fields we’re looking at are parameters to this function. And because the function here is static, they’re not instance members either. I assume AV_MessagesTexte is basically a global of text labels, which isn’t a bad way to manage such a thing, but functions should still take those globals as parameters so you can test them.

I’m kidding, of course. This function has never been tested.

Aside from a gigantic pile of string length comparisons, what does this function actually do? Well, it returns a new string which is a number of spaces exactly equal to the length of the longest string. And the way we build that output string is not only through string concatenation, but the use of a while loop where a for loop makes more sense.

Also, just… why? Why do we need a spaces-only-string the length of another string? Even if we’re trying to do some sort of text layout, that seems like a bad way to do whatever it is we’re doing, and also if that’s the case, why is it called getPrefix? WHY IS OUR PREFIX A STRING OF SPACES THE LENGTH OF OUR FIELD? HOW IS THAT A PREFIX?

I feel like I’m going mad.

But the real star of this horrible mess, in my opinion, is the exception handling. Get an exception? Show the user a form! There’s no attempt to decide if or how we could recover from this error, we just annoy the user with it.

Which isn’t just unique to this function. Notice the getmaxLength function? It’s really a max and it looks like this:

private static int getmaxLength(int old, int current)
{
	int result = old;
	try
	{
		if (current > old)
		{
			result = current;
		}
	}
	catch (Exception error)
	{
		ErrorForm frm = new ErrorForm(error);
		frm.ShowDialog();
	}
	return result;
}

What’s especially delightful here is that this function couldn’t possibly throw an exception. And you know what that tells me? This try/catch/form pattern is just their default error handling. They spam this everywhere, in every function, and the tech lead or architect pats themselves on the back for ensuring that the application “never crashes!” all the while annoying the users with messages they can’t do anything about.

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