One of the lines between code that's "not great, but like, it's fine, I guess" and "wow, WTF" is confidence.

For example, Francis Gauthier inherited a WinForms application. One of the form fields in this application was a text box that held a number, and the developers wanted to always display whatever the user entered without leading zeroes.

Now, WinForms is pretty simplistic as UI controls go, so there isn't really a great "oh yes, do this!" solution to solving that simple problem. A mix of using MVC-style patterns with a formatter between the model and the UI would be "right", but might be more setup than the problem truly calls for.

Which is why, at first blush, without more context, I'd be more apt to put this bad code into the "not great, but whatever" category:

   int percent = Int32.Parse(ctrl.Text);
   ctrl.Text = percent.ToString();

On an update, we grab the context of the text box, parse it as an integer, and then store the result back into the text box. This will effectively strip off the leading zeroes.

It's fine. Until we zoom out a step.

 // Matched - Remove leading zero
try {
   int percent = Int32.Parse(ctrl.Text);
   ctrl.Text = percent.ToString();
}
catch {
 // impossible..
}

Here, we can see that they… "wisely" have wrapped the Parse in an exception handler. The developer knew that there was a validator on the control which would prevent non-numeric characters from being entered, and thus they were able with a great degree of confidence to declare that an exception was "impossible".

There's just one problem with that. The validator in question allows numeric characters, not just integer characters. So the validator would allow you to enter 0.99. Which of course, won't parse. So the exception gets triggered, the catch ignores it, the user believes their input- a percentage- has been accepted as valid. The end result is that many users might enter "0.99" to mean "99%", and then see "0" be what actually gets stored as the unexpected floating point gets truncated.

All because an exception was declared "impossible". To misapply a quote: "You keep using that word. I do not think it means what you think it means."

[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/an-impossible-problem

"No, that's not the coupon code. They literally ran out of digital coupons," Steve wrote.

"Wow! Amazon is absolutely SLASHING their prices out of existence," wrote Brian.

Bj"orn S. writes, "IKEA now employs what I'd call psychological torture kiosks. The text translates to 'Are you satisfied with your waiting time?' but the screen below displays an eternal spinner. Gaaah!"

Daniel O. writes, "I mean, I could change my password right now, but I'm kind of tempted to wait and see when it'll actually expire."

"This bank seems to offer its IT employees some nice perks! For example, this ATM is reserved strictly for its administrators," Oton R. wrote.

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

https://thedailywtf.com/articles/nothing-for-you

Cloud management consoles are, in most cases, targeted towards enterprise customers. This runs into Remy’s Law of Enterprise Software: if a piece of software is in any way described as being “enterprise”, it’s a piece of garbage.

Richard was recently poking around on one of those cloud provider’s sites. The software experience was about as luxurious as one expects, which is to say it was a pile of cryptically named buttons for the 57,000 various kinds of preconfigured services this particular service had on offer.

At the bottom of each page, there was a small video thumbnail, linking back to a YouTube video, presumably to provide marketing information, or support guidance for whatever the user was trying to do.

This was the code which generated them (whitespace added for readability):

I appreciate the use of a tag. Providing a meaningful fallback for browsers that, for whatever reason, aren’t executing JavaScript is a good thing. In the olden days, we called that “progressive enhancement” or “graceful degradation”: the page might work better with JavaScript turned on, but you can still get something out of it even if it’s not.

Which is why it’s too bad the is being output by JavaScript.

And sure, I’ve got some serious questions with the data-lazy-src attribute, the fact that we’re dumping a div with a class="play" presumably to get wired up as our play button, and all the string mangling to get the correct video ID in there for the thumbnail, and just generating DOM elements by strings at all. But outputting s from JavaScript is a new one on me.

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

https://thedailywtf.com/articles/graceful-depredations

TypeScript offers certain advantages over JavaScript. Compile time type-checking can catch a lot of errors, it can move faster than browsers, so it offers the latest standards (and the compiler handles the nasty details of shimming them into browsers), plus it has a layer of convenient, syntactic sugar.

If you’re using TypeScript, you can use the compiler to find all sorts of ugly problems with your code, and all you need to do is turn the right flags on.

Or, you can be like Quintus’s co-worker, who checked in this… thing.

/**
 * Container holding definition information.
 *
 * @param String version
 * @param String date

 */
export class Definition {
  private id: string;
  private name: string;
  constructor(private version, private data) {}

  /**
   * get the definition version
   *
   * @return String version
   */
  getVersion() {
    return this.id;
  }

  /**
   * get the definition date
   *
   * @return String date
   */
  getDate() {
    return this.name;
  }
}

Now, if you were to try this on the TypeScript playground, you’d find that while it compiles and generates JavaScript, the compiler has a lot of reasonable complaints about it. However, if you were just to compile this with the command line tsc, it gleefully does the job without complaint, using the default settings.

So the code is bad, and the tool can tell you it’s bad, but you have to actually ask the tool to tell you that.

In any case, it’s easy to understand what happened with this bad code: this is clearly programming by copy/paste. They had a class that tied an id to a name. They copy/pasted to make one that mapped a version to a date, but got distracted halfway through and ended up with this incomplete dropping. And then they somehow checked it in, and nobody noticed it until Quintus was poking around.

Now, a little bit about this code. You’ll note that there are private id and name properties. The constructor defines two more properties (and wires the constructor params up to map to them) with its private version, private data params.

So if you call the constructor, you initialize two private members that have no accessors at all. There are accessors, but they point to id and name, which never get initialized in the constructor, and have no mutators.

Of course, TypeScript compiles down into JavaScript, so those private keywords don’t really matter. JavaScript doesn’t have private.

My suspicion is that this class ended up in the code base, but is never actually used. If it is used, I bet it’s used like:

let f = new Definition();
f.id = "1.0.1"
f.name = "28-OCT-2020"
…
let ver = f.getVersion();

That would work and do what the original developer expected. If they did that, the TypeScript compiler might complain, but as we saw, they don’t really care about what the compiler says.

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

https://thedailywtf.com/articles/a-type-of-useless

Understanding the Gang of Four design patterns is a valuable bit of knowledge for a programmer. Of course, instead of understanding them, it sometimes seems like most design pattern fans just… use them. Sometimes- often- overuse them. The Java Spring Framework infamously has classes with names like SimpleBeanFactoryAwareAspectInstanceFactory. Whether that's a proper use of patterns and naming conventions is a choice I leave to the reader, but boy do I hate looking at it.

The GoF patterns break down into four major categories: Behavioral, Structural, Concurrency, and Creational patterns. The Creational category, as the name implies, is all about code which can be used to create instances of objects, like that Factory class above. It is a useful collection of patterns for writing reusable, testable, and modular code. Most Dependency Injection/Inversion of Control frameworks are really just applied creational patterns.

It also means that some people decide that "directly invoking constructors is considered harmful". And that's why Emiko found this Java code:

	/**
	 * Creates an empty {@link MessageCType}.
	 * @return {@link MessageCType}
	 */
	public static MessageCType createMessage() {
		MessageCType retVal = new MessageCType()
		return retVal;
	}

This is just a representitive method; the code was littered with piles of these. It'd be potentially forgiveable if they also used a fluent interface with method chaining to intialize the object, buuut… they don't. Literally, this ends up getting used like:

	MessageCType msg = MessageCType.createMessage();
	msg.type = someMessageType;
	msg.body = …

Emiko sums up:

At work, we apparently pride ourselves in using the most fancyful patterns available.

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

https://thedailywtf.com/articles/on-the-creation

If we presume there is a Hell for IT folks, we can only assume the eternal torment involves configuring or interfacing with printers. Perhaps Anabel K is already in Hell, because that describes her job.

Anabel's company sells point-of-sale tools, including receipt printers. Their customers are not technical, so a third-party installer handles configuring those printers in the field. To make the process easy and repeatable, Anabel maintains an app which automates the configuration process for the third party.

The basic flow is like this:

The printer gets shipped to the installer. At their facility, someone from the installer opens the box, connects the printer to power, and then to the network. They then run the app Anabel maintains, which connects to the printer's on-board web server and POSTs a few form-data requests. Assuming everything works, the app reports success. The printer goes back in the box and is ready to get installed at the client site at some point in the near future.

The whole flow was relatively painless until the printer manufacturer made a firmware change. Instead of the username/password being admin/admin, it was now admin/serial-number. No one was interested in having the installer techs key in the long serial number, but digging around in the documentation, Anabel found a simple fix.

In addition to the on-board web-server, there was also a TCP port running. If you connected to the port and sent the correct command, it would reply with the serial number.

Anabel made the appropriate changes. Now, her app would try and authenticate as admin/admin, and if it failed, it'd open a TCP connection, query the serial number, and then try again. Anabel grabbed a small pile of printers from storage, a mix of old and new firmware, loaded them up with receipt paper, and ran the full test suite to make sure everything still worked.

Within minutes, they were all happily churning out test prints. Anabel released her changes to production, and off it went to the installer technicians.

A few weeks later, the techs call in through support, in an absolute panic. "The configuration app has stopped working. It doesn't work on any of the printers we received in the past few weeks."

There was a limited supply of the old version of printers, and dozens got shipped out every day. If this didn't get fixed ASAP, they would very quickly find themselves with a pile of printers the installers couldn't configure. Management got on conference calls, roped Anabel in on the middle of long email chains, and they all agreed: there must be something wrong with Anabel's changes.

It wasn't unreasonable to suspect, but Anabel had tested it thoroughly. Heck, she had a few of the new printers on her desk and couldn't replicate the failure. So she got on a call with a tech and started from square one. Is it plugged in. Is it plugged into the network. Are there any restrictions on the network, or on the machine running the app, that might prevent access to non-standard ports?

Over the next few days, while the stock of old printers kept dwindling, this escalated up to sending a router with a known configuration out to the technicians. It was just to ensure that there were no hidden firewalls or network policies preventing access to the TCP port. Even still, on its own dedicated network, nothing worked.

"Okay, let's double check the printer's network config," Anabel said on the call. "When it boots up, it should print out its network config- IP, subnet, gateway, DHCP config, all of that. What does that say?"

The tech replied, "Oh. We don't have paper in it. One sec." While rooting around in the box, they added, "We don't normally bother. It's just one more thing to unbox before putting it right back in the box."

The printer booted up, faithfully printed out its network config, which was what everyone expected. "Okay, I guess… try running the tool again?" Anabel suggested.

And it worked.

Anabel turned to one of the test printers she had been using, and pulled out the roll of receipt paper. She ran the configuration tool… and it failed.

The TCP service only worked when there was paper in the printer. Anabel reported it as a bug to the printer vendor, but if and when that gets fixed is anyone's guess. The techs didn't want to have to fully unbox the printers, including the paper, for every install, but that was an easy fix: with each shipment of printers Anabel's company just started shipping a few packs of receipt paper for the techs. They can just crack one open and use it to configure a bunch of printers before it runs out.

[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/serial-problems

"I can understand selling swiss cheese by the slice, but copier paper by the pound?" Dave P. wrote.

Amanda R. writes, "Ok, that's fine, but can the 1% correctly spell 'people'?"

"In this form, language is quite variable as is when you are able to cancel your reservation ...which are in fact, actual variables," wrote Jean-Pierre M.

Barry M. wrote, "Hey, Royal Caribbean, you know what? I'll take the win-win: total control AND save $7!"

"Oh wow! The secret on how to write good articles is out!" writes Barry L.

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

https://thedailywtf.com/articles/errors-by-the-pound

It’s generally hard to do worse than a SQL injection vulnerability. Data access is fundamental to pretty much every application, and every programming environment has some set of rich tools that make it easy to write powerful, flexible queries without leaving yourself open to SQL injection attacks.

And yet, and yet, they’re practically a standard feature of bad code. I suppose that’s what makes it bad code.

Gidget W inherited a PHP application which, unsurprisingly, is rife with SQL injection vulnerabilities. But, unusually, it doesn’t leverage string concatenation to get there. Gidget’s predecessor threw a little twist on it.

$fields = "t1.id, t1.name, UNIX_TIMESTAMP(t1.date) as stamp, ";
$fields .= "t2.idT1, t2.otherDate, t2.otherId";
$join = "table1 as t1 join table2 as t2 on t1.id=t2.idT1";
$where = "where t1.lastModified > $val && t2.lastModified = '$val2'";
$query = "select $field from $join $where";

This pattern appears all through the code. Because it leverages string interpolation, the same core structure shows up again and again, almost copy/pasted, with one line repeated each time.

$query = "select $field from $join $where";

What goes into $field and $join and $where may change each time, but "select $field from $join $where" is eternal, unchanging, and omnipresent. Every database query is constructed this way.

It’s downright elegant, in its badness. It simultaneously shows an understanding of how to break up a pattern into reusable code, but also no understanding of why all of this is a bad idea.

But we shouldn’t let that distract us from the little nuances of the specific query that highlight more WTFs.

t1.lastModified > $val && t2.lastModified = '$val2'

lastModified in both of these tables is a date, as one would expect. Which raises the question: why does one of these conditions get quotes and why does the other one not? It implies that $val probably has the quotes baked in?

Gidget also asks: “Why is the WHERE keyword part of the $where variable instead of inline in the query, but that isn’t the case for SELECT or FROM?”

That, at least, I can answer. Not every query has a filter condition. Since you can’t have WHERE followed by nothing, just make the $where variable contain that.

See? Elegant in its badness.

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

https://thedailywtf.com/articles/query-elegance

About three years ago, Consuela inherited a giant .NET project. It was… not good. To communicate how “not good” it was, Consuela had a lot of possible submissions. Sending the worst code might be the obvious choice, but it wouldn’t give a good sense of just how bad the whole thing was, so they opted instead to find something that could roughly be called the “median” quality.

This is a stored procedure that is roughly about the median sample of the overall code. Half of it is better, but half of it gets much, much worse.

CREATE proc [dbo].[usermgt_DeleteUser]
  (
    @ssoid uniqueidentifier
  )
AS
  begin
    declare @username nvarchar(64)
    select @username = Username from Users where SSOID = @ssoid
    if (not exists(select * from ssodata where ssoid = @ssoid))
      begin
        insert into ssodata (SSOID, UserName, email, givenName, sn)
        values (@ssoid, @username, 'Email@email.email', 'Firstname', 'Lastname')
        delete from ssodata where ssoid = @ssoid
      end
    else begin
      RAISERROR ('This user still exists in sso', 10, 1)
    end

Let’s talk a little bit about names. As you can see, they’re using an “internal” schema naming convention- usermgt clearly is defining a role for a whole class of stored procedures. Already, that’s annoying, but what does this procedure promise to do? DeleteUser.

But what exactly does it do?

Well, first, it checks to see if the user exists. If the user does exist… it raises an error? That’s an odd choice for deleting. But what does it do if the user doesn’t exist?

It creates a user with that ID, then deletes it.

Not only is this method terribly misnamed, it also seems to be utterly useless. At best, I think they’re trying to route around some trigger nonsense, where certain things happen ON INSERT and then different things happen ON DELETE. That’d be a WTF on its own, but that’s possibly giving this more credit than it deserves, because that assumes there’s a reason why the code is this way.

Consuela adds a promise, which hopefully means some follow-ups:

If you had access to the complete codebase, you would not EVER run out of new material for codesod. It’s basically a huge collection of “How Not To” on all possible layers, from single lines of code up to the complete architecture itself.

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

https://thedailywtf.com/articles/delete-this

The C# "extension method" feature lets you implement static methods which "magically" act like they're instance methods. It's a neat feature which the .NET Framework uses extensively. It's also a great way to implement some convenience functions.

Brandt found some "convenience" functions which were exploiting this feature.

public static bool IsLessThen(this T a, T b) where T : IComparable
		=> a.CompareTo(b) < 0;

public static bool IsGreaterThen(this T a, T b) where T : IComparable
		=> a.CompareTo(b) > 0;

public static bool And(this bool a, bool b)
		=> a && b;

public static bool Or(this bool a, bool b)
		=> a || b;

public static bool IsBetweensOrEqual(this T a, T b, T c) where T : IComparable
		=> a.IsGreaterThen(b).Or(a.Equals(b)).And(
				a.IsLessThen(c).Or(a.Equals(c))
				);

Here, we observe someone who maybe heard the term "functional programming" and decided to wedge it into their programming style however it would fit. We replace common operators and expressions with extension method versions. Instead of the cryptic a || b, we can now write a.Or(b), which is… better?

I almost don't hate the IsLessThan/IsGreaterThan methods, as that is (arguably) more readable. But wait, I have to correct myself: IsLessThen and IsGreaterThen. So they almost got something that was (arguably) more readable, but with a minor typo just made it all more confusing.

All this, though, to solve their actual problem: the TimeSpan data type doesn't have a "between" comparator, and at one point in their code- one point- they need to perform that check. It's also worth noting that C# supports operator overloading, and TimeSpan does have an overload for all your basic comparison operators, so they could have just done that.

Brandt adds:

While you can argue that there is no out of the box 'Between' functionality, the colleague who programmed this ignored an already existing extension method that was in the same file, that offered exactly this functionality in a better way.

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

https://thedailywtf.com/articles/extended-time

One of my favorite illusions is the progress bar. Even the worst, most inaccurate progress bar will make an application feel faster. The simple feedback which promises "something is happening" alters the users' sense of time.

So, let's say you're implementing a JavaScript progress bar. You need to decide if you are "in progress" or not. So you need to check: if there is a progress value, and the progress value is less than 100, you're still in progress.

Mehdi's co-worker decided to implement that check as… the opposite.

const isInProgress = progress => !(!progress || (progress && progress > 100))

This is one of those lines of code where you can just see the developer's process, encoded in each choice made. "Okay, we're not in progress if progress doesn't have a value: !(!progress). Or we're not in progress if progress has a value and that value is over 100."

There's nothing explicitly wrong with this code. It's just the most awkward, backwards possible way to express that check. I suspect that part of its tortured logic arises from the fact that the developer wanted to return false if the value was null or undefined, and this was the way they figured out to do that.

Of course, a more straightforward way to write that might be (progress) => (progress && progress <= 100) || false. This will have "unexpected" behavior if the progress value is negative, but then again, so will the original code.

In the end, this is just a story of a double negative. I definitely won't say you should never not use a double negative. Don't not avoid them.

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

https://thedailywtf.com/articles/don-t-not-be-negative

https://thedailywtf.com/articles/try-a-different-address

Mapping between types can create some interesting challenges. Michal has one of those scenarios. The code comes to us heavily anonymized, but let’s see what we can do to understand the problem and the solution.

There is a type called ItemA. ItemA is a model object on one side of a boundary, and the consuming code doesn’t get to touch ItemA objects directly, it instead consumes one of two different types: ItemB, or SimpleItemB.

The key difference between ItemB and SimpleItemB is that they have different validation rules. It’s entirely possible that an instance of ItemA may be a valid SimpleItemB and an invalid ItemB. If an ItemA contains exactly five required fields importantDataPieces, and everything else is null, it should turn into a SimpleItemB. Otherwise, it should turn into an ItemB.

Michal adds: “Also noteworthy is the fact that ItemA is a class generated from XML schemas.”

The Java class was generated, but not the conversion method.

public ItemB doConvert(ItemA itemA) {
  final ItemA emptyItemA = new ItemA();
  emptyItemA.setId(itemA.getId());
  emptyItemA.setIndex(itemA.getIndex());
  emptyItemA.setOp(itemA.getOp());

  final String importantDataPiece1 = itemA.importantDataPiece1();
  final String importantDataPiece2 = itemA.importantDataPiece2();
  final String importantDataPiece3 = itemA.importantDataPiece3();
  final String importantDataPiece4 = itemA.importantDataPiece4();
  final String importantDataPiece5 = itemA.importantDataPiece5();

  itemA.withImportantDataPiece1(null)
          .withImportantDataPiece2(null)
          .withImportantDataPiece3(null)
          .withImportantDataPiece4(null)
          .withImportantDataPiece5(null);

  final boolean isSimpleItem = itemA.equals(emptyItemA) 
&& importantDataPiece1 != null && importantDataPiece2 != null && importantDataPiece3 != null && importantDataPiece4 != null;

  itemA.withimportantDataPiece1(importantDataPiece1)
          .withImportantDataPiece2(importantDataPiece2)
          .withImportantDataPiece3(importantDataPiece3)
          .withImportantDataPiece4(importantDataPiece4)
          .withImportantDataPiece5(importantDataPiece5);

  if (isSimpleItem) {
      return simpleItemConverter.convert(itemA);
  } else {
      return itemConverter.convert(itemA);
  }
}

We start by making a new instance of ItemA, emptyItemA, and copy a few values over to it. Then we clear out the five required fields (after caching the values in local variables). We rely on .equals, generated off that XML schema, to see if this newly created item is the same as our recently cleared out input item. If they are, and none of the required fields are null, we know this will be a SimpleItemB. We’ll put the required fields back into the input object, and then call the appropriate conversion methods.

Let’s restate the goal of this method, to understand how ugly it is: if an object has five required values and nothing else, it’s SimpleItemB, otherwise it’s a regular ItemB. The way this developer decided to perform this check wasn’t by examining the fields (which, in their defense, are being generated, so you might need reflection to do the inspection), but by this unusual choice of equality test. Create an empty object, copy a few ID related elements into it, and your default constructor should handle nulling out all the things which should be null, right?

Or, as Michal sums it up:

The intention of the above snippet appears to be checking whether itemA contains all fields mandatory for SimpleItemB and none of the other ones. Why the original author started by copying some fields to his ‘template item’ but switched to the ‘rip the innards of the original object, check if the ravaged carcass is equal to the barebones template, and then stuff the guts back in and pretend nothing ever happened’ approach halfway through? I hope I never find out what it’s like to be in a mental state when any part of this approach seems like a good idea.

Ugly, yes, but still, this code worked… until it didn’t. Specifically, a new nullable Boolean field was added to ItemA which was used by ItemB, but had no impact on SimpleItemB. This should have continued to work, except that the original developer defaulted the new field to false in the constructor, but didn’t update the doConvert method, so equals started deciding that our input item and our “empty” copy no longer matched. Downstream code started getting invalid ItemB objects when it should have been getting valid SimpleItemB objects, which triggered many hours of debugging to try and understand why this small change had such cascading effects.

Michal refactored this code, but was not the first person to touch it recently:

A cherry on top is the fact that importantDataPiece5 came about a few years after the original implementation. Someone saw this code, contributed to the monstrosity, and happily kept on trucking.

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

https://thedailywtf.com/articles/a-new-generation

Janell found herself on a project where most of her team were offshore developers she only interacted with via email, and a tech lead who had not ever programmed on the .NET Framework, but did some VB6 “back in the day”, and thus “knew VB”.

The team dynamic rapidly became a scenario where the tech lead issued an edict, the offshore team blindly applied it, and then Janell was left staring at the code wondering how to move forward with this.

These decrees weren’t all bad. For example: “Avoid repeated code by re-factoring into methods,” isn’t the worst advice. It’s not complete advice- there’s a lot of gotchas in that- but it fits on a bumper-sticker and generally leads to better code.

There were other rules that… well:

To improve the performance of the garbage collector, all variables must be set to nothing (null) at the end of each method.

Any time someone says something like “to improve the performance of the garbage collector,” you know you’re probably in for a bad time. This is no exception. Now, in old versions of VB, there’s a lot of “stuff” about whether or not you need to do this. It was considered a standard practice in a lot of places, though the real WTF was clearly VB in this case.

In .NET, this is absolutely unnecessary, and there are much better approaches if you need to do some sort of cleanup, like implementing the IDisposable interface. But, since this was the rule, the offshore team followed the rule. And, since we have repeated code, the offshore team followed that rule too.

Thus:

Public Sub SetToNothing(ByVal object As Object)
    object = Nothing
End Sub

If there is a platonic ideal of bad code, this is very close to that. This method attempts to solve a non-problem, regarding garbage collection. It also attempts to be “DRY”, by replacing one repeated line with… a different repeated line. And, most important: it doesn’t do what it claims.

The key here is that the parameter is ByVal. The copy of the reference in this method is set to nothing, but the original in the calling code is left unchanged in this example.

Oh, but remember when I said, “they were attempting to be DRY”? I lied. I’ll let Janell explain:

I found this gem in one of the developers classes. It turned out that he had copy and pasted it into every class he had worked on for good measure.

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

https://thedailywtf.com/articles/nothing-but-garbage

After years spent supporting an enterprisey desktop application with a huge codebase full of WTFs, Sammy thought he had seen all there was to be seen. He was about to find out how endlessly deep the bottom of the WTF barrel truly was.

During development, Sammy frequently had to restart said application: a surprisingly onerous process, as it took about 30 seconds each and every time to return to a usable state. Eventually, a mix of curiosity and annoyance spurred him into examining just why it took so long to start.

He began by profiling the performance. When the application first initialized, it performed 10 seconds of heavy processing. Then the CPU load dropped to 0% for a full 16 seconds. After that, it increased, pegging out one of the eight cores on Sammy's machine for 4 seconds. Finally, the application was ready to accept user input. Sammy knew that, for at least some of the time, the application was calling out to and waiting for a response from a server. That angle would have to be investigated as well.

Further digging and hair-pulling led Sammy to a buried bit of code, a Very Old Mechanism for configuring the visibility of items on the main menu. While some menu items were hard-coded, others were dynamically added by extension modules. The application administrator had the ability to hide or show any of them by switching checkboxes in a separate window.

When the application first started up, it retrieved the user's latest configuration from the server, applied it to their main menu, then sent the resulting configuration back to the server. The server, in turn, called DELETE * FROM MENU_CFG; INSERT INTO MENU_CFG (…); INSERT INTO MENU_CFG (…); …

Sammy didn't know what was the worst thing about all this. Was it the fact that the call to the server was performed synchronously for no reason? Or, that after multiple DELETE / INSERT cycles, the table of a mere 400 rows weighed more than 16 MB? When the users all came in to work at 9:00 AM and started up the application at roughly the same time, their concurrent transactions caused quite the bottleneck—and none of it was necessary. The Very Old Mechanism had been replaced with role-based configuration years earlier.

All Sammy could do was write up a change request to put in JIRA. Speaking of bottlenecks ...

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

https://thedailywtf.com/articles/slow-load

Date representations are one of those long-term problems for programmers, which all ties into the problem that "dates are hard". Nowadays, we have all these fancy date-time types that worry about things like time zones and leap years, and all of that stuff for us. Pretty much everything, at some level, relies on the Unix Epoch. But there is a time before that was the standard.

In the mainframe era, not all the numeric representations worked the same way that we're used to, and it was common to simply define the number of digits you wanted to store. So, if you wanted to store a date, you could define an 8-digit field, and store the date as 20201012: October 10th, 2020.

This is a pretty great date format, for that era. Relatively compact (and yes, the whole Y2K thing means that you might have defined that as a six digit field), inherently sortable, and it's not too bad to slice it back up into date parts, when you need it. And like anything else which was a good idea a long time ago, you still see it lurking around today. Which does become a problem when you inherit code written by people who didn't understand why things worked that way.

Virginia N inherited some C# code which meets that criteria. And the awkward date handling isn't even the WTF. There's a lot to unpack in this particular sample, so let's start with… the unpack function.

public static DateTime UnpackDateC(DateTime dateI, long sDate)
{
    if (sDate.ToString().Length != 8) return dateI;

    try
    {
        return new DateTime(Convert.ToInt16(sDate.ToString().Substring(0, 4)),
            Convert.ToInt16(sDate.ToString().Substring(4, 2)),
            Convert.ToInt16(sDate.ToString().Substring(6, 2)));
    }
    catch
    {
        return dateI;
    }
}

sDate is our integer date: 20201012. Instead of converting it to a string once, and then validating and slicing it, we call ToString four times. We also reconvert each date part back into an integer so we can pass them to DateTime, and of course, DateTime.ParseExact is just sitting there in the documentation, shaking its head at all of this.

The really weird choice to me, though, is that we pass in dateI, which appears to be the "fallback" date value. That… worries me.

Well, let's take a peek deep in the body of a method called GetMC, because that's where this unpack function is called.

while (oDataReader.Read())
{
    //...
    DataRow oRow = oDataTable.NewRow();

    if (oDataReader["DEB"] != DBNull.Value)
    {
        DateTime dt = DateTime.Today;
        dt = UnpackDateC(dt, Convert.ToInt64(oDataReader["DEB"]));
    }
    else
    {
        oRow["DEB"] = DBNull.Value;
    }
    //...
}

It's hard to know for absolute certain, based on the code provided, but I don't think UnpackDateC is actually doing anything. We can see that the default dateI value is DateTime.Today. So perhaps the desired behavior is that every invalid/unknown date is today? Seems problematic, but maybe that jives with the requriements.

But note the logic. If the database value is null, we store a null in oRow["DEB"]- our output data. If it isn't null, we unpack the date and store it in… dt. Also, if you trace the type conversions, we convert an integer in the database into an integer in our program (which it already would have been) so that we can convert that integer into a string so that we can split the string and convert each portion into integers so we can convert it into a date.

How do I know that the field is an integer in the database? Well, I don't know for sure, but let's look at the query which drives that loop.

public static void GetMC(string sConnectionString, ref DataTable dtToReturn, string sOrgafi,
    string valid, string exe, int iOperateur,
    string sColTri, bool Asc, bool DBLink, string alias) // iOperateur 0, 1, 2
{
sSql =
        " select * from (select ENTLCOD as ENTAFI, MARKYEAR as EXE, nvl(to_char(MARKNUM),'')||'-'||nvl(MARKNUMLOT,'') as COD, to_char(MARKNUM) as NUM, MARKOBJ1 as COM, MARKSTARTDATE as DEB, MARKENDDATE as FIN, MARKNUMLOT as NUMLOT, MARKVALIDDATE, TIERNUM as FOUR, MARTBASTIT as TYP " +
        " from SOMEEXTERNALVIEW" + (DBLink ? alias + " " : " ") + " WHERE 1=1";
        if (valid != null && valid.Length > 0)
        sSql += " and (MARKVALIDDATE >= " + valid + " or MARKVALIDDATE=0 or MARKVALIDDATE is null)";
    if (exe != null && exe.Length > 0) sSql += " and TRIM( MARKYEAR ) ='" + exe.Trim() + "' ";
    sSql += " ) where 1=1";
    //...

We can see that MARKSTARTDATE is the database field we call DEB. We can also see some conditional string concatenation to build our query, so hello possible SQL injection attacks. Now, I don't know that MARKSTARTDATE is an integer, but I can see that a similar field, MARKVALIDDATE is. Note the lack of quotes in the query string: "…(MARKVALIDDATE >= " + valid + " or MARKVALIDDATE=0 or MARKVALIDDATE is null)"

So MARKVALIDDATE is numeric in the database, which is great because the variable valid is passed in as a string, so we're just all over the place with types.

The structure of this query also adds on an extra layer of unnecessary complexity, as for some reason, we wrap the actual query up as a subquery, but the outer query is just SELECT * FROM (subquery) WHERE 1=1, so there is literally no reason to do that.

To finish this off, let's look at where GetMC is actually invoked, a method called CallWSM.

private void CallWSM(ref DataTable oDataTable, string sCode, string sNom, string sFourn, int iOperateur)   // iOperateur 0, 1, 2
{
    try
    {
        m_bError = false;
        string sColTri = m_Grid_SortRequest.FieldName;
        SortOperator oDirection = m_Grid_SortRequest.SortDirection;
        m_sAnnee = ctlRecherche.GetValueFilterItem(1);
        string svalid = "";
        string sdt = ctlRecherche.GetValueDateItem(0);
        if (sdt.Length > 0)
        {
            DateTime dtvalid = DateTime.Parse(sdt);
            long ldt = dtvalid.Year * 10000 + dtvalid.Month * 100 + dtvalid.Day;
            svalid = ldt.ToString();
        }
        m_AppLogic.GetMC(sColTri, oDirection, m_sAdresseWS, ref oDataTable, svalid, m_sAnnee, iOperateur, PageCurrent, m_PageSize);
    }
    catch (WebException ex)
    {
        if (ex.Status == WebExceptionStatus.Timeout)
        {
            frmMessageBox.Show(ML.GetLibelle(4137), CONST.AppTITLE, MessageBoxButtons.OK, MessageBoxIcon.Error);
            m_bError = true;
        }
        else
        {
            frmMessageBox.Show(ex.Message);
            m_bError = true;
        }
    }
    catch (Exception ex)
    {
        frmMessageBox.Show(ex.Message);
        m_bError = true;
    }
}

Now, I'm reading between the lines a bit, and maybe making some assumptions that I shouldn't be, but this method is called CallWSM, and one of the parameters we pass to GetMC is stored in a variable called m_sAdresseWS, and GetMC can apparently throw a WebException.

Are… are we building a query and then passing it off to a web service to execute? And then wrapping the response in a data reader? Because that would be terrible. But if we're not, does that mean that we're also calling a web service in some of the code Virginia didn't supply? Query the DB and call the web service in the same method? Or are we catching an exception that just could never happen, and all the WS stuff has nothing to do with web services?

Any one of those options would be a WTF.

Virginia adds, "I had the job to make a small change in the call. ...I'm used to a good amount of Daily WTF-erry in our code." After reading through the code though, Virginia had some second thoughts about changing the code. "At this point I decided not the change anything, because it hurts my head."

You and me both.

[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/intergral-to-dating

"Boy oh boy! You can't beat that free shipping for the low, low price of $4!" Zenith wrote.

Brendan wrote, "So, as of September 24th, my rent is both changing and not changing, in the future, but also in the past?"

"Does waiting for null people take forever or no time at all?" Pascal writes.

"Whoever at McDonalds is responsible for calculating their cheeseburger deals needs to lay off of the special sauce," Valts S. wrote.

"So, assuming I were to buy 0.9 of a figure, what's the missing 10%?" writes Zenith.

[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/math-is-hard

Content management systems are… an interesting domain in software development. On one hand, they’re one of the most basic types of CRUD applications, at least at their core. On the other, it’s the sort of problem domain where you can get 90% of what you need really easily, but the remaining 10% are the cases that are going to make you pull your hair out.

Which is why pretty much every large CMS project supports some kind of plugin architecture, and usually some sort of marketplace to curate those plugins. That kind of curation is hard, writing plugins tends to be more about “getting the feature I need” and less about “releasing a reliable product”, and thus the available plugins for most CMSes tend to be of wildly varying quality.

Paul recently installed a plugin for Joomla, and started receiving this error:

Row size too large. The maximum row size for the used table type, not counting BLOBs, is 8126. This includes storage overhead, check the manual. You have to change some columns to TEXT or BLOBs

Paul, like a good developer, checked with the plugin documentation to see if he could fix that error. The documentation had this to say:

you may have too many languages installed

This left Paul scratching his head. Sure, his CMS had eight different language packs installed, but how, exactly, was that “too many”? It wasn’t until he checked into the database schema that he understood what was going on:

This plugin’s approach to handling multiple languages was simply to take every text field it needed to track and add a column to the table for every language. The result was a table with 231 columns, most of them language-specific duplicates.

Now, there are a few possible reasons why this is this way. It may simply be whoever wrote the plugin didn’t know or care about setting up a meaningful lookup table. Maybe they were thinking about performance, and thought that “well, if I denormalize I can get more data with a single query”. Maybe they weren’t thinking at all. Or maybe there’s some quirk about creating your own tables as a Joomla plugin that the developer didn’t want to create more tables than absolutely necessary.

Regardless, it’s definitely one of the worst schemas you could come up with to handle localization.

Paul adds: “I vote the developer for Best Database Designer of 2020.”

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

https://thedailywtf.com/articles/translation-by-column

At an old job, I did a significant amount of VB.Net work. I didn’t hate VB.Net. Sure, the syntax was clunky, but autocomplete mostly solved that, and it was more OR
less feature-matched to C# (and, as someone who needed to handle XML, the fact that VB.Net had XML literals was handy).

Every major feature in C# had a VB.Net equivalent, including lambdas. And hey, lambdas are great! What a wonderful way to express a filter condition.

Well, Eric O sends us this filter lambda. Originally sent to us as a single line, I’m adding line breaks for readability, because I care about this more than the original developer did.

Function(row, index) index <> 0 AND
(row(0).ToString().Equals("DIV10106") OR
row(0).ToString().Equals("326570") OR
row(0).ToString().Equals("301100") OR
row(0).ToString().Equals("305622") OR
row(0).ToString().Equals("305623") OR
row(0).ToString().Equals("317017") OR
row(0).ToString().Equals("323487") OR
row(0).ToString().Equals("323488") OR
row(0).ToString().Equals("324044") OR
row(0).ToString().Equals("317016") OR
row(0).ToString().Equals("316875") OR
row(0).ToString().Equals("323976") OR
row(0).ToString().Equals("324813") OR
row(0).ToString().Equals("147000") OR
row(0).ToString().Equals("326984") OR
row(0).ToString().Equals("326634") OR
row(0).ToString().Equals("306039") OR
row(0).ToString().Equals("307021") OR
row(0).ToString().Equals("307050") OR
row(0).ToString().Equals("307603") OR
row(0).ToString().Equals("307604") OR
row(0).ToString().Equals("307632") OR
row(0).ToString().Equals("307704") OR
row(0).ToString().Equals("308184") OR
row(0).ToString().Equals("308531") OR
row(0).ToString().Equals("309930") OR
row(0).ToString().Equals("104253") OR
row(0).ToString().Equals("104532") OR
row(0).ToString().Equals("104794") OR
row(0).ToString().Equals("104943") OR
row(0).ToString().Equals("105123") OR
row(0).ToString().Equals("105755") OR
row(0).ToString().Equals("106075") OR
row(0).ToString().Equals("108062") OR
row(0).ToString().Equals("108417") OR
row(0).ToString().Equals("108616") OR
row(0).ToString().Equals("108625") OR
row(0).ToString().Equals("108689") OR
row(0).ToString().Equals("108851") OR
row(0).ToString().Equals("108997") OR
row(0).ToString().Equals("109358") OR
row(0).ToString().Equals("109551") OR
row(0).ToString().Equals("110081") OR
row(0).ToString().Equals("111501") OR
row(0).ToString().Equals("111987") OR
row(0).ToString().Equals("112136") OR
row(0).ToString().Equals("11229") OR
row(0).ToString().Equals("112261") OR
row(0).ToString().Equals("113127") OR
row(0).ToString().Equals("113266") OR
row(0).ToString().Equals("114981") OR
row(0).ToString().Equals("116527") OR
row(0).ToString().Equals("121139") OR
row(0).ToString().Equals("121469") OR
row(0).ToString().Equals("142449") OR
row(0).ToString().Equals("144034") OR
row(0).ToString().Equals("144693") OR
row(0).ToString().Equals("144900") OR
row(0).ToString().Equals("150089") OR
row(0).ToString().Equals("194340") OR
row(0).ToString().Equals("214950") OR
row(0).ToString().Equals("215321") OR
row(0).ToString().Equals("215908") OR
row(0).ToString().Equals("216531") OR
row(0).ToString().Equals("217151") OR
row(0).ToString().Equals("220710") OR
row(0).ToString().Equals("221265") OR
row(0).ToString().Equals("221387") OR
row(0).ToString().Equals("300011") OR
row(0).ToString().Equals("300013") OR
row(0).ToString().Equals("300020") OR
row(0).ToString().Equals("300022") OR
row(0).ToString().Equals("300024") OR
row(0).ToString().Equals("300026") OR
row(0).ToString().Equals("300027") OR
row(0).ToString().Equals("300050") OR
row(0).ToString().Equals("300059") OR
row(0).ToString().Equals("300060") OR
row(0).ToString().Equals("300059") OR
row(0).ToString().Equals("300125") OR
row(0).ToString().Equals("300139") OR
row(0).ToString().Equals("300275") OR
row(0).ToString().Equals("300330") OR
row(0).ToString().Equals("300342") OR
row(0).ToString().Equals("300349") OR
row(0).ToString().Equals("300355") OR
row(0).ToString().Equals("300363") OR
row(0).ToString().Equals("300413") OR
row(0).ToString().Equals("301359") OR
row(0).ToString().Equals("302131") OR
row(0).ToString().Equals("302595") OR
row(0).ToString().Equals("302621") OR
row(0).ToString().Equals("302649") OR
row(0).ToString().Equals("302909") OR
row(0).ToString().Equals("302955") OR
row(0).ToString().Equals("302986") OR
row(0).ToString().Equals("303096") OR
row(0).ToString().Equals("303249") OR
row(0).ToString().Equals("303753") OR
row(0).ToString().Equals("304010") OR
row(0).ToString().Equals("304016") OR
row(0).ToString().Equals("304047") OR
row(0).ToString().Equals("304566") OR
row(0).ToString().Equals("305347") OR
row(0).ToString().Equals("305486") OR
row(0).ToString().Equals("305487") OR
row(0).ToString().Equals("305489") OR
row(0).ToString().Equals("305526") OR
row(0).ToString().Equals("305568") OR
row(0).ToString().Equals("305769") OR
row(0).ToString().Equals("305773") OR
row(0).ToString().Equals("305824") OR
row(0).ToString().Equals("305998") OR
row(0).ToString().Equals("306039") OR
row(0).ToString().Equals("307021") OR
row(0).ToString().Equals("307050") OR
row(0).ToString().Equals("307603") OR
row(0).ToString().Equals("307604") OR
row(0).ToString().Equals("307632") OR
row(0).ToString().Equals("307704") OR
row(0).ToString().Equals("308184") OR
row(0).ToString().Equals("308531") OR
row(0).ToString().Equals("309930") OR
row(0).ToString().Equals("322228") OR
row(0).ToString().Equals("121081") OR
row(0).ToString().Equals("321879") OR
row(0).ToString().Equals("327391") OR
row(0).ToString().Equals("328933") OR
row(0).ToString().Equals("325038")) AND DateTime.ParseExact(row(2).ToString(), "dd.MM.yyyy", System.Globalization.CultureInfo.InvariantCulture).CompareTo(DateTime.Today.AddDays(-14)) <= 0

That is 5,090 characters of lambda right there, clearly copy/pasted with modifications on each line. The original developer, at no point, paused to think a moment about whether or not this was the way to achieve their goal?

If you’re wondering about those numeric values, I’ll let Eric explain:

The magic numbers are all customer object references, except the first one, which I have no idea what is.

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

https://thedailywtf.com/articles/a-long-time-to-master-lambdas

In a global health crisis, like say, a pandemic, accurate and complete data about its spread is a "must have". Which is why, in the UK, there's a great deal of head-scratching over how the government "lost" thousands of cases.

Oops.

Normally, we don't dig too deeply into current events on this site, but the circumstances here are too "WTF" to allow them to pass without comment.

From the BBC, we know that this system was used to notify individuals if they have tested positive for COVID-19, and notify their close contacts that they have been exposed. That last bit is important. Disease spread can quickly turn exponential, and even though COVID-19 has a low probability of causing fatalities, the law of large numbers means that a lot of people will die anyway on that exponential curve. If you can track exposure, get exposed individuals tested and isolated before they spread the disease, you can significantly cut down its spread.

People are rightfully pretty upset about this mistake. Fortunately, the BBC has a followup article discussing the investigation, where an analyst explores what actually happened, and as it turns out, we're looking at an abuse of everyone's favorite data analytics tool: Microsoft Excel.

The companies administering the tests compile their data into plain text which appear to be CSV files. No real surprise there. Each test created multiple rows within the CSV file. Then, the people working for Public Health England imported that data into Excel… as .xls files.

.xls is the old Excel format, dating back into far-off years, and retained for backwards compatibility. While modern .xlsx files can support a little over a million rows, the much older format caps out at 65,536.

So: these clerks imported the CSV file, hit "save as…" and made a .xls, and ended up truncating the results. With the fact that these input datasets had multiple rows per tests, "in practice it meant that each template was limited to about 1,400 cases."

Again, "oops".

I've discussed how much users really want to do everything in Excel, and this is clearly what happened here. The users had one tool, Excel, and it looked like a hammer to them. Arcane technical details like how many rows different versions of Excel may or may not support aren't things it's fair to expect your average data entry clerk to know.

On another level, this is a clear failing of the IT services. Excel was not the right tool for this job, but in the middle of a pandemic, no one is entirely sure what they needed. Excel becomes a tempting tool, because pretty much any end user can look at complicated data and slice/shape/chart/analyze it however they like. There's a good reason why they want to use Excel for everything: it's empowering to the users. When they have an idea for a new report, they don't need to go through six levels of IT management, file requests in triplicate, and have a testing and approval cycle to ensure the report meets the specifications. They just… make it.

There are packaged tools that offer similar, purpose built functionality but still give users all the flexibility they could want for slicing data. But they're expensive, and many organizations (especially government offices) will be stingy about who gets a license. They may or may not be easy to use. And of course, the time to procure such a thing was in the years before a massive virus outbreak. Excel is there, on everyone's computer already, and does what they need.

Still, they made the mistake, they saw the consequences, so now we know, they will definitely start taking steps to correct the problem, right? They know that Excel isn't fit-for-purpose, so they're switching tools, right?

From the BBC:

To handle the problem, PHE is now breaking down the data into smaller batches to create a larger number of Excel templates in order to make sure none hit their cap.

Oops.

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

https://thedailywtf.com/articles/excellent-data-gathering