We're heading into the holiday season. We had some rather ambitious plans for a special Christmas event, but for a variety of reasons, all the various moving parts couldn't come together.

There's always next year, though. Instead of rehashing Christmas carols or doing our own parodies of holiday specials, I figured we should share one of the songs we wrote for that unfinished special event. It's got good advice for those of us heading into the holiday season: don't release anything to prod in the next few days.

Lyrics

Git Pull

CHORUS:
Git pull,
Don't push,
(JOLLY BOSS)But merge to prod? 
    (All)No way!
Build it,
Test it,
Our long break starts today.

VERSE
With serverless and lambdas,
Failover sites galore,
In Atlanta, Sri Lanka, Alabama and Casablanca,
Our apps crash no more.

Our on-prem is monitored,
Our logs are all Splunked,
A horde of comps and cords up on our big board,
Holiday failures are flunked

CHORUS
Git pull,
Don't push,
(JOLLY BOSS)Just one quick patch? 
    (All) No way!
Build it,
Test it,
Our long break starts today!

VERSE
Holidays are near us,
We've paid off our tech debt,
(DEV 1) There's no muss, 
    (DEV 2) (Not a) fuss, 
        (DEV 3) to make us cuss,
(All)Everything is set

CHORUS
Git pull,
Don't push,
(JOLLY BOSS)Just change the font? 
    (All) NO WAY!
Build it,
Test it,
Our long break starts today!

Lyrics - Remy Porter and James Rushin
Music - James Rushin
"Git Pull" Vocalists - Brianna Barnes, Billy Eric Robinson, Beda Spindola, Sarah Rose Shaiman, Ethan Lynch

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

https://thedailywtf.com/articles/git-pull-for-the-holidays

Dirk's employer bought some software from a vendor, and the vendor promised that it was a cutting edge system, which was pushing the boundaries of technology. Digging through the API documentation, Dirk definitely found some unusual innovations, like the way it tracked log levels:

[FlagsAttribute]
public enum TraceEventType
{
    NoTracing = 0,
    Critical = 1,
    Error = 2,
    Warning = 4,
    Information = 8,
    Verbose = 16
}

Your boring old software treats log levels as, well, levels. A message exists on a level, you filter for a certain level, so if you only want Error messages, you won't see Debug messages, but if you Debug you'll also see Errors.

This cutting edge system lets you decide what message you want to see, because message levels are bitmasks. You want critical information? Look for 9. You want only critical warnings, whatever that may actually be? 5 it is. And does a 6 tell you that the warning was an error, or that there was an error outputting the warning?

And yes, most methods that involved logging let you log a message with any integer combination, and most that involved filtering logs let you filter with a bitmask. Unfortunately, not all of them did, and the behavior of "filter by bitmask" was inconsistent, as most of the logging engine assumed you wanted to filter by levels, so a filter of 17 might, in some cases, only show critical messages.

Such is the path of true pioneers: their brilliant idea is held back by systems that are entrenched in the "old" way of doing things. Or maybe someone just didn't really understand how log levels and enums were supposed to work and just made a bitmask by accident.

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

https://thedailywtf.com/articles/masking-errors

"It's a fix for a fix for a fix, apparently," writes Joan.

In this case, it's some Ruby code, which… gets a bit repetitive in that it repeats itself and does the same thing over and over again, in a repetitive fashion. Repeats.

    def index
        payment_service_parameters = { external_type: PaymentService::CANCEL_TYPES, state: 'pending' }
        payment_service_parameters.merge!(params.fetch(:transfer_groups, {})) if params[:transfer_groups].present?

        payment_service_parameters = { state: 'pending', external_type: PaymentService::CANCEL_TYPES }
        if params[:transfer_groups].present?
          payment_service_parameters = params.fetch(:transfer_groups, {})
          payment_service_parameters[:external_type].reject!(&:blank?)
          payment_service_parameters[:external_type] = PaymentService::CANCEL_TYPES if payment_service_parameters[:external_type].blank?
        end

        render locals: { transfer_groups: load_transfer_groups(payment_service_parameters) }
    end

We start by populating the payment_service_parameters property with a default value. Then, we check the params collection and, if it has a :transfer_groups key, we update the payment_service_parameters. Then we blow that away and reset the payment_service_parameters back to the original value.

Then we check for :transfer_groups in the params collection again, and if it exists we replace the entirety of payment_service_parameters with :transfer_groups, but then we make sure the key :external_type has a value.

This is the sort of code that clearly was written by someone who didn't know what the code was supposed to do and just kept adding lines until the errors go away. Joan agrees:

Nobody asked what should the code do. The more you look at it, the worse it gets. Every check is done three times. But there are still several possible errors.

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

https://thedailywtf.com/articles/again-and-again

Mike's co-worker doesn't like to write code that gets tightly coupled to one system. Everything should be completely abstracted so that, if something on the backend changes, your code doesn't need to change.

Which is an admirable goal, perhaps, but is also premature abstraction. For example, Mike's company uses Asterisk, an open-source PBX system. This co-worker wanted to script a few actions in that system, but didn't want his scripts to be tightly coupled to the system that he was scripting, so he… overgeneralized. But also didn't. But also did it wrong.

Specifically, new voicemails needed to also get sent to an email address. Asterisk stores voicemails in the form msgNNNN.wav, where NNNN is a 0-padded numeric value, e.g. msg0123.wav.

This is the PHP code to find the proper file to send:

class SendVoicemailEmail {
//...
	private function getMessageNumberForAttachmentFilename()
	{
		return $this->getMessageNumber() - 1;
	}

	private function getAttachmentFileName()
	{
		foreach (array('INBOX', 'Urgent') as $vmPriorityDir) {
			$voicemailAudioDir = $this->getVoicemailDirectory($vmPriorityDir);
			$files = scandir($voicemailAudioDir);
			foreach ($files as $file) {
				if ($this->isCorrectFileName($file)) {
					return $this->getVoicemailDirectory($vmPriorityDir) . $file;
				}
			}
		}
		return false;
	}

	private function isCorrectFileName($fileName)
	{
		$fileName = strtolower($fileName);
		$suffix = $this->getMessageNumberForAttachmentFilename() . $this->getAttachmentSuffix();
		$suffixLocation = strpos($fileName, $suffix);
		if ($suffixLocation !== false) {
			$fileName = str_replace($suffix, '', $fileName);
			$fileName = str_replace($this->getAttachmentPrefix(), '', $fileName);
			if (is_numeric($fileName) && ($fileName == 0)) return true;
		}
		return false;
	}
//...
}

The first function, getMessageNumberForAttachmentFilename just addresses the fact that the filenames are zero-indexed, but the metadata is one-indexed, so that's pretty reasonable.

The second function, getAttachmentFileName is where things start to drift off the rails. You see, if you just used the Asterisk naming convention in this code, you could just grab the file you needed. But that would mean encoding the specific vendor's naming convention into this method, which the developer didn't want to do. So instead, the code scans every file in two directories to see if one of them is the "correct" filename.

But it's the third method, isCorrectFileName where things really lose control. First, we calculate the $suffix, which is the message number combined with the file extension, e.g. 123.wav. We attempt to find that substring in the filename, and if it exists, we strip it out, along with msg. Finally, if it's a numeric value and equal to zero, we've found the correct filename.

There are so many things going wrong here. First, this hasn't really abstracted out the Asterisk filename convention anyway, so the entire goal of making this super abstract was missed. But the Asterisk file naming convention allows for up to 10,000 files (0000-9999), while this developer's approach only supports 1,000.

Say the filename is msg1234.wav. We calculate the suffix, 1234.wav, and remove that from the string, leaving msg. We then script the prefix, leaving an empty string. According to is_numeric, an empty string is not numeric.

This bug was discovered, of course, when the system started believing new voicemails stopped existing. But not right away, there were a few days of confused and angry users who perhaps were puzzled about why they weren't getting new voicemails, but weren't sure what to make of it.

Mike decided the easiest fix was to take the risk of tightly coupling to Asterisk's filename convention and just load the file directly, without scanning any directories.

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

https://thedailywtf.com/articles/voicemail-to-email-to-nothing-at-all

Alas, I have only the following five submissions to share, as my controversy generator is on the Fritz today.

Ex post autumnal Adam R. warmly reflects "I'm not quite sure how the math works out here, but I'm apparently a one-percenter in the home energy savings department."

Unfamous Ross wonders

Trustworthy and fully-named Joshua Herzig-Marx is buying a bundle of sweatpants. It's winter in his nabe, too. "For some reason, Amazon needs to create this weird digital download in order to sell me a two-pack of sweatpants. Don't worry, if you download it - it's not a virus! For real!"

'nother Northerner Joerg S. explains the wtferry below thusly: "Microsoft uses machine translation on Bing to the extreme. The harmless Tangens function (tan on your calculator button) became 'Gelbbraun' in the translation to German. Of course, 'Gelbbraun' is the correct translation for the color 'tan'. But why would a calculator button be labeled with the name of a color? I reported this error with the bing feedback function a few weeks ago, but maybe I used the wrong font color?

Classics fan Dave P. replays an old tune. "Let's see. I can buy the MP3s for this album for $24.99. Or I can buy the MP3s for $24.99, and you'll pay me $13.11 for the privilege of shipping the physical CD to me." Sure, Dave, or there's another option for even a few bucks cheaper.

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

https://thedailywtf.com/articles/pop

At various points in my career, I spent my days delivering technical training. If your company wanted to adopt a new technology, or upskill your developers, you'd hire me and I'd do crash course sessions. I generally didn't design course material: the books, slide decks, and labs. I just delivered the course. Most of the course material I worked with was pretty good, but there were some which were clearly written by people who were just trying to get a course out the door fast while a new technology was buzzwordy enough that nobody knew what they were buying.

I hated those courses.

All this means that we're going to shift gears a little bit into something we don't usually look at: the kinds of code that may be used as examples in training material.

The first one comes from Jan, who was just trawling through JavaScript guides, and stumbled across one article that really stressed the importance of comments. For example, this incredibly complex method needs this comment attached to it:

/**
 Saves the user to the database.
 It receives a "User" object, it generates a random alphanumeric ID and then it tries to save it into the users table.
 If sucessfull, it'll return the new generated user ID.
 If it fails, it'll return "false" instead.
*/
function saveUser(usr) {
  usr.id = Math.random().toString(36).substring(2);
  let result = usr.save();
  if(!result) {
    return false;
  } 
  return usr.id;
}

There's no big WTF here, but this code is meant to be educational, which is why I'm picking on it.

There's the initial premise of this code being so complicated in needs a thorough comment that explains line-by-line what the code does. That's clearly not true, though this code definitely needs documentation, because one needs to understand what it returns when.

But that's also the problem. Error states generally shouldn't propagate through return values if your language has another option. A version of this method that simply returns the ID or throws an exception on failure would both be easier to understand and easier to document.

Of course, "a bad, lazy code sample," is par for the course in a lot of educational materials, so this is just our appetizer.

Steve Wahl signed up for some security training. He was quite happy to see C offered as an option, since he's used to seeing very web-focused security training and since he does Linux kernel and driver development, that's not exactly useful.

What wasn't clear was that they expected developers to be ready to delve into Windows APIs as well as Linux APIs. Many of the exercises took the form of "spot the vulnerability", so take a look at this block and see if you can guess the right answer:

// Do NOT forget to call '_freea(stackBuf);'
void* StackAllocate(const size_t size)
{
    if (size < 1U || size > 1024U * 1024U)
        return NULL;

    void* buf = NULL;

#ifdef _WIN32
    buf = _malloca(size);
#else
    buf = alloca(size);
#endif
    return buf;
}

What the training wanted you to spot was that on Windows, _malloca can raise a stack overflow error, which needs to be handled with Windows' "structured exception handling" dialect. This code doesn't handle that error correctly, and thus, that's the danger.

Which, sure, not handling exceptions is bad. But that's hardly the thing that we should be paying attention to.

First off, alloca always allocates memory on the current stack frame. Which means when StackAllocate returns, you're returning a pointer to memory you don't own. Don't do this. That is a bad thing to do.

But the Windows case, with _malloca gets weirder, because _malloca doesn't guarantee that it allocates memory on the stack. It might acquire heap memory. Which is probably why the comment warns you need to call _free- alloca is "automatically" cleaned up (along with the stack frame), while _malloca can't be automatically cleaned up.

Regardless, both of these methods might actually allocate stack memory, in which case you can't safely pass back a pointer from either of them. That's the actual bug in the code, not the missing exception handling.

If this code existed in production, it'd be a complete WTF. That it's used to illustrate mistakes made that might create security problems, it's missing the forest for the trees. This is a clear case that the person designing the training had a very specific understanding of very specific things, but didn't assemble them into a bigger picture to actually communicate the lesson to their students- students who, by their very nature, might not understand what's going on well enough to recognize how misleading the educational content is.

I promise not to pick on training materials too much, but especially the secure coding example really bugged me.

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

https://thedailywtf.com/articles/a-learning-opportunity

https://thedailywtf.com/articles/don-t-lookup-the-log4j-debacle

Many years ago, Brian was on the lookout for a new contract. While hunting, he found an opportunity to maintain some Intranet software for a large European news agency. He contacted them, and had a series of conversations with one of the managers.

"You see," the manager explained, "Sven was the code guru behind this entire system. He was… a colorful personality, but extremely smart. We don't have a lot of confidence that we could just slot a contractor into his role and expect them to be able to deliver what Sven could deliver for us."

To give them that confidence, Brian agreed to do some very small contract work before they moved on to a larger, longer-term contract. This let him see exactly what Sven could deliver.

	
    $req000="SELECT idform FROM form WHERE idevent=".$_GET['id_evt'];
    $rep000=$db4->query($req000);
    $nb000=$rep000->numRows();
    if ($nb000>0) {
        while ($row000=$rep000->fetchRow(DB_FETCHMODE_ASSOC)) {
            $req001="UPDATE form SET idevent=NULL WHERE idform=".$row000['idform'];
            $rep001=$db4->query($req001);
        }
    }

The first thing Brian pointed out to his prospective employer that, as written, this was vulnerable to SQL injection attacks right from the address bar. That alone gave the manager a fright and may have been what clinched a large contract for Brian to continue cleaning up this code.

But a SQL injection vulnerability isn't a WTF. So it's helpful that everything else about this is awful.

The goal of this block of code is to find all of the forms where idevent has a certain value and null that field out. To do this, we do a select from form to find all the idforms where that's the case, then we can iterate across that list to UPDATE the form table.

Which, of course, could all be done in a single update: UPDATE form SET idevent NULL WHERE idevent=".$_GET["id_evt"]. I've left the SQL injection vulnerability in there, just to keep at least some of the WTF in place.

But it's the variable names that are the fascinating part. Sort of a monstrous version of hungarian notation where the prefix gives you a vague sense of the role of the variable: nb for "number", req for a database "request", rep for a… reply? And then the unique name of the variable is… its numeric component. That we see $db4 in here implies some other horrors lurking just outside of this block.

Brian writes:

"Code gurus" like this keep me employed so I should be thankful. It made me laugh, then cry, and then laugh again.

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

https://thedailywtf.com/articles/art000

Daniel inherited some code which depends heavily on stored procedures. The person who developed those stored procedures was incredibly fond of overengineering things. Incredibly fond.

The result is a big pile of not incredibly interesting T-SQL code. I'll share the whole block of code in a moment, but there's one specific comment that I want to highlight before we look at it:

	----Always try to update your temp table..in the event there is a error on your logic
	----you don't update a bunch of PROD recs in error

Don't touch data in real tables, until after touching the data in a temp table, just in case you make a mistake. This is how coding works, right? If you just add extra steps it'll stop mistakes, somehow, magically? This highlights the core logical errors.

CREATE PROCEDURE [dbo].[***_*****_Savecasesentstatus] 
			 @Xml XML
AS

/*
Created By: ************
Date: 11/26/2020
Use: Get the status on sent **** cases (initial or an update).  Then inserts the records into table ****_*****_CaseSendVendorAuthStatus.
Date: 12/03/2020 - adding code to check for errors.  Don't want to wrap code in a Transaction for risk of locking up table *****.dbo.mm360_caseBase.
				 I will use the @@ERROR option. 	
*/

	DECLARE @RecCount INT;
	SET NOCOUNT OFF;
	WITH CTE
		AS (SELECT CaseRecordID = XTbl.value('(CaseRecordID)[1]', 'nvarchar(200)')
			    , Payor = XTbl.value('(Payor)[1]', 'varchar(5)')
		    FROM @Xml.nodes('/NewDataSet/Table1') AS XD(XTbl))
		SELECT LTRIM(RTRIM(CaseRecordID)) AS CaseRecordID
			, Payor
			, CONVERT(VARCHAR(25), '') AS   CaseSendStatus
		INTO #RecsFromSentFile
		FROM CTE;
	IF @@Error > 0
	    BEGIN
		   SELECT @@Error AS ErrorNum;
		   RETURN;
	END;
	SELECT a.CaseRecordID
		, a.Payor
		, a.CaseSendStatus
		, ISNULL(b.mm360_VendorAuthStatus, 'Null Found') AS mm360_VendorAuthStatus
	INTO #OrigRecsWithVendAuthStatus
	FROM #RecsFromSentFile AS a
		INNER JOIN
		******.dbo.mm360_caseBase AS b
		ON a.CaseRecordID COLLATE SQL_Latin1_General_CP1_CI_AS = b.mm360_recordid COLLATE SQL_Latin1_General_CP1_CI_AS;
	IF @@Error > 0
	    BEGIN
		   SELECT @@Error AS ErrorNum;
		   RETURN;
	END;

	----Always try to update your temp table..in the event there is a error on your logic
	----you don't update a bunch of PROD recs in error

	UPDATE a
	  SET 
		 a.CaseSendStatus = CASE LTRIM(RTRIM(b.mm360_VendorAuthStatus))
						    WHEN ''
						    THEN 'Initial'
						    WHEN 'Null Found'
						    THEN 'Initial'
						    WHEN 'Initial Error'
						    THEN 'Initial'
						    WHEN 'Update Error'
						    THEN 'Update'
						    WHEN 'Finalized'
						    THEN 'Update'
						    WHEN 'Pending'
						    THEN 'Pending'
						    WHEN 'Voided'
						    THEN 'Voided'
						END
	FROM #RecsFromSentFile a
		INNER JOIN
		#OrigRecsWithVendAuthStatus b
		ON a.CaseRecordID = b.CaseRecordID;
	IF @@Error > 0
	    BEGIN
		   SELECT @@Error AS ErrorNum;
		   RETURN;
	END;

	-- Update present cases for same day...If multple file runs have took place ..exclude the previous cases
	-- to keep the integrity of data for reporting
	UPDATE a
	  SET 
		 a.Exclude = 1
	FROM ****_*****_278_Sent_History a
		INNER JOIN
		#RecsFromSentFile b
		ON a.CaseRecordID = b.CaseRecordID
	WHERE CONVERT(VARCHAR(10), a.EnterDate, 101) = CONVERT(VARCHAR(10), GETDATE(), 101);

	----Store historical reference	 
	INSERT INTO ****_****_278_Sent_History
	(CaseRecordID
    , Payor
    , VendorAuthStatus
    , EnterDate
    , Exclude
	)
	SELECT CaseRecordID
		, Payor
		, CaseSendStatus
		, GETDATE()
		, 0
	FROM #RecsFromSentFile;
	SELECT @@RowCount AS TotalInsertedRecs;
	DROP TABLE #RecsFromSentFile;
	DROP TABLE #OrigRecsWithVendAuthStatus;

Now, some of this overcomplexity is because the they "Don't want to wrap code in a Transaction for risk of locking up table *****.dbo.mm360_caseBase". Of course, you can control when the transaction starts, so they could always do their read operation without acquiring locks if they don't really care, and then use transactions for the actual updates. Right off the bat, we get the sense that someone understands transactions have a cost and a tradeoff, but doesn't understand what that actually is or the correct way to manage that in SQL Server.

From there, we populate temp tables- the first with parsed XML data, the second with a quin back to that mm360_caseBase. Then we munge that data with some updates, many of which could have probably been done as part of the initial queries.

We also are doing this on temp tables, again, because we don't want to "update a bunch of PROD recs in error". Which is why we then go and update a bunch of prod records in 278_Sent_History to set the Exclude flag in the very next step- so that the new records we're adding can have an Exclude of 0.

How much of this is actually necessary? Turns out, pretty much none of it. The XML file actually contains much of the data they're joining to get, the whole Exclude flag thing is already handled by the downstream code without ever checking the Exclude flag. Daniel was able to rewrite this as a single "read from the XML and insert into the target table" without having to jump through all of those hoops.

And finally, as a minor pet peeve, since the temp tables are prefixed with #, they're local temp tables, which means they're auto-dropped as soon as they go out of scope. So the DROP TABLE statements at the end aren't needed.

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

https://thedailywtf.com/articles/just-a-temporary-thing

Admittedly, this holiday season is culturally dominated by the Anglo-American tradition of a wintertime Christmas. Currier & Ives notwithstanding, most of the various holidays obviously originated in some summer event, or else a harvest one. For these hot holidays, our mate Peter starts things off with a very warm greeting from down under. Following that are a few of my favorite stocking stuffers, until Peter's antipodal ally Michael closes everything out with an appeal to warm the heart but not blister the sole.

Aussie Peter G. mutters weakly "I think this warning is bit superfluous (for the non-metric, that's 550 degrees F)."

Regular contributor Pascal comes in strong with a caroling round.

Echoing Pascal's refrain, Mike S. chimes "So does this mean the error has an error which has an error?"

While sadly demoralized Daniel simply moans "This was attempt number three. I have since given up trying to change my address. Or determine the existence or otherwise of the error. Or anything else."

But be of good cheer, Daniel. A year ago, belated reveler Michael R. brought us a story from the UK about Xmas Jumper Day. He'd missed his opportunity but found an intriguing Err. Apparently, there were more submissions that week, or funnier ones, or the stars were not aligned appropriately, and we didn't run his entry at that time. But imagine my surprise when reviewing past submissions to realise that today is Jumper Day! So with that synchronicity, here is what Michael left under our 2020 tree, and a rare earnest moment.

Unfortunately, it's too late for you to sign up to raise funds now and to participate in all the joy that extroverts undoubtedly derive from dressing up in ugly outfits, but if you want to check out what Jumper Day really is and support a worthy cause, you can start here.

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

https://thedailywtf.com/articles/some-like-it-hot

On one hand, we should all be doing our revision tracking in our source control system, and we shouldn't let code comments tell the story of why or how the code changed. On the other, when it leaves us the history of WTFs in the current version of the code, it's actually delightful.

Take this anonymous submission, which includes the whole story of the code, all at once.

if (sw ==null)
{
        sw = new Stopwatch();
        ResetStopwatch();
}

// Was the following: the purpose was: check if there is a stopwatch, if not, do the create one and reset it.
// the code above seems simpeler

//try
//{
//    bool check = sw.IsRunning;
//}
//catch
//{
//    sw = new Stopwatch();
//    ResetStopwatch();
//}

The original version, commented out, uses a null reference exception for control flow. If sw.IsRunning throws an exception (because sw is null), we catch it and create a new stopwatch.

Which is certainly more complicated and annoying than a simple null check. And the fix was "simpel". And we have the whole narrative of the original, why it changed, and what it is now.

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

https://thedailywtf.com/articles/stop-watch

Different languages will frequently have similar syntax. Many a pointy-haired-boss has seen this similarity and thus assumed that, if their programmer knows C, then C++ should be easy, and if they know C++ then going into C# must be trivial. I mean, the languages look the same, so they must be the same, right? Boats and cars are steered by wheels, so clearly if you can drive a car you can pilot a boat, and nothing will go wrong.

Andreas S inherited some code that started at C/C++ and was then ported to C#. The original developers were the sort to reinvent wheels wherever possible, so it's no surprise that they kept that going when they moved into C#.

There are, for example, a few methods not supplied in today's code sample. They wrote their own RoundTo, which Andreas describes thus: "RoundTo() is basically equal to Math.Round(), but implemented in a WTF way." There is also a homebrew Sprintf implemented as an "extension method" on Strings.

These all get combined in the method GetStringToRound which doesn't get anything, but seems to just format a string into a rounded off value.

public static string GetStringToRound(double dValue, int iDecimals)
{
    double dDummy = dValue;
    string sFormat = $"%.{iDecimals}lf";
    double dDummyRound = 0.0;
    string sReturn = "";

    for (int i = 0; i <= iDecimals; i++)
    {
        dDummyRound = RoundTo(dDummy, i);
        double dDifferenz = RoundTo(dDummy - dDummyRound, i + 1);

        if (dDifferenz == 0.0)
        {
            sFormat = sFormat.Sprintf("%ld", i);
            sFormat = "%." + sFormat + "lf";
            break;
        }
    }

    if (dValue != dDummyRound)
    {
        dValue = dDummyRound;
    }

    sReturn = sReturn.Sprintf(sFormat, dValue);
    return sReturn;
}

I think the dummy here is me, because I do not understand any of the logic going on inside that for loop. Why is it even a for loop? I see that we're checking if rounding to i and i+1 decimal places are the same, we know that we don't really need to round farther. Which, sure… but… why?

I'm sure this code works, and I'm sure it's doing what it was intended to do. I also know that there are built-in methods which already do all of this, that are cleaner and easier to read and understand, and don't leave be scratching my head feeling like a dDummy.

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

https://thedailywtf.com/articles/dummy-round

Anita, a senior developer, had recently been hired at a company with around 60 employees. Her first assignment was to assist with migrating the company’s flagship on-premises application to the cloud. After a year of effort, the approach was deemed unworkable and the entire project was scrapped. Seem a little hasty? Well, to be fair, the company made more money selling the servers and licenses for running their application on-premise than they made on the application itself. Multiple future migration attempts would meet the same fate, but that's a whole other WTF.

With the project's failure, Anita became redundant and feared being let go. Fortunately, the powers-that-be transferred her to another department instead. This was when Anita first met Henry, her new manager.

Henry was a database guy. It wasn't clear how much project management experience he had, but he definitely knew a great deal about business intelligence and analytics. Henry explained to Anita that she’d be working on implementing features that customers had been requesting for years. Anita had never worked with analytics before, but did have a background in SQL databases. She figured multi-dimensional databases shouldn't be too hard to learn. So learn she did, working as a one-person army. Henry never put any time pressure on her, and was always happy to answer her questions. The only downside to working for him was his disdain toward open source solutions. Anita couldn't use any NuGet packages whatsoever; everything had to be built from scratch. She learned a ton while making her own JSON parsing library and working out OAuth 2.0 authentication.

Upon completing a project, Anita would go to Henry's office to demo it. Once satisfied, Henry would say "Great!" and hand her a new project. Whenever Anita asked if there were a feature request logged somewhere that she could close out, she was told she didn't have to worry about it. She would also ask about her previous efforts, whether they'd been tested and released. Henry usually replied along the lines of, "I haven't had time yet, but soon!"

Over time, Anita noticed an uncomfortable trend: every 12 months or so, the higher-ups fired 20-30% of the entire staff, normally from Sales or Marketing. The company wasn't making as much money as the shareholders thought it should, so they would fire people, wait a few months, then hire new people. For the most part, though, they spared the developers working on the core application. They were smart enough to understand that no one coming in cold would be capable of figuring out this beast.

She figured she was safe—and yet, after 6 years, Anita found herself being fired. Henry brought her into his office, insisted it wasn't his choice, and emphasized that he was very sorry to lose her.

Anita shook her head in resignation. After the shock had worn off, she'd looked into a few things. "Over the years, you gave me 8 projects to work on," she said.

Henry nodded.

"Are you aware that you never tested any of them?" Anita asked. "You never released them to production. They're still sitting in SourceSafe, waiting for someone to use them. Are you aware that the company has never seen a penny from the work I've done for you?"

From the look on his face, it was clear that Henry had never realized this.

"This has been a good learning experience, at least," Anita said. "Thanks for everything."

Anita was able to take the knowledge she'd gained and double her salary at her next job, which only took 3 weeks to find. She's no longer reinventing the wheel or going unappreciated, and that's a win-win for sure.

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

https://thedailywtf.com/articles/unseen-effort

Scientists frequently need software to support their research, but rarely are strong developers. And why should they be, that software is written to accomplish a goal, and it's the goal which matters to them more than anything about the software itself.

That's where Jared comes in. He worked in a university IT department and his job was simply to write the software the researchers needed. They frequently had a very clear picture of what they needed, along with big piles of math to explain it, plus piles of example input and expected output data.

The team was small- just Jared and two other developers, Larry and Barry. There was no team lead, they could simply coordinate and divide the work. Their manager was nearly invisible, and mostly focused on keeping external office politics from disrupting the work. The work wasn't precisely simple, but it was clear and well defined, the pay was good.

While the code quality wasn't perfect, it was good enough. Since throughput was one of the main drivers of the design, a lot of code segments got hyperoptimized in ways that made the code harder to understand and maintain, but boosted its performance. The software was mostly stateless, the unit test coverage was nearly 100%, and while the UI was the ugliest of ducklings, the software worked and gave the researches what they wanted.

In short, it was the best job Jared had ever had up to that point.

Six months into the job, Jared came into work as he normally did. Larry was an early starter, so he was already at his desk, but instead of typing away, he simply sat at his desk. His hands were in his lap, and Larry simply stared forlornly at his keyboard. Barry had a similar thousand-yard stare aimed at his monitor.

"What's wrong?" Jared asked.

The answer was just a long sigh followed by, "Just… go pull the latest code."

So Jared did. The latest commit eradicated everything. Most of the files in the codebase had been deleted and replaced with a single 7,000 line God Object. And the git blame told him the culprit, someone named "Scott".

"Who the heck is Scott?"

Larry just shook his head and sighed again. Barry chimed in. "He's our technical lead."

"Wait, we got a new technical lead?" Jared asked.

"No, he's always been the tech-lead. Apparently. I had to check the org-chart because I didn't even know- none of us did. But Larry's worked with Scott before."

That revelation caused Larry to sigh again, and shake his head.

"It apparently didn't go well," Barry explained.

A little later that morning, Scott wandered into their team room. "Hey, Josh, you must be the new developer."

"Jared, and I've been here for six months-"

"So, I noticed we'd gotten a little off track," Scott said, ignoring Jared. "I've been bouncing around a lot of different projects, because well, you know, when you're good people want you to do everything, and work miracles, amirite? Anyway, we're going to make some changes to improve code quality and overall design."

So Scott walked them through his new architectural pattern. That 7,000 line God Object was, in part, an event bus. No object was allowed to talk directly to any other object. Instead, it needed to raise an event and let the bus pass the information to the next object. For "performance" each event would spawn a new thread. And since the God Object contained most of the application logic, most events on the bus were sent from, and received by, the God Object.

As a bonus, Scott hadn't written any unit tests. There were more compiler warnings than lines of code- Scott's code averaged 1.3 warnings per line of code. And no, Scott would not allow anyone to revert back to the old code. He was the tech-lead, by god, and he was going to lead them.

Also, 18-person months of work had just been deleted, all the new features that Jared, Larry and Barry had added over the past six months. Their end users were furious that they'd just lost huge pieces of functionality. Also, the multi-threaded "performant" version took hours to do things that used to take seconds. And within a few weeks of Scott's "revisions" the number of bugs tracked in Jira rose by a factor of six.

The researchers did not like this, and simply refused to update to the new application. Scott did not like that, and started trying to force them to change. Their manager didn't like any of this, and pressured the team to fix the new version of the application. And quickly, which meant a lot of unpaid overtime. Once overtime started, Scott "surprisingly" got pulled into another project which needed his attention.

With Scott gone, they were almost able to revert to the old version of the code, but there had been so much work put into the new version that their manager only saw the sunk costs and not the bigger picture. They were committed to sinking the ship, whether they liked it or not.

Bug counts kept rising, it got harder and harder to implement new features, there was a scramble to hire new people and the team of three suddenly became a team of twelve, but nothing could be done for it. The system had become an unmaintainable Codethulhu.

The best job Jared had ever had turned into the worst, all with one commit from Scott. Jared quit and found a new job. Scott kept plugging away, vandalizing codebases in the university for another five years before finally screwing up big enough to get fired. The last Jared saw on LinkedIn, Scott had moved on to another, larger, and more prestigious university, doing the same development work he had been doing.

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

https://thedailywtf.com/articles/leading-from-affronts

This week, anonymous Franz starts us off with a catch-22. "I opened MS Word (first time after reboot) and a dialog box opens that tells me to close a dialog box. This was the only open dialog box... I guess even software makes excuses to be lazy on Fridays."

Sarcastic Laks exclaims "Too much speed!" but declines further wit. "I'll leave that snarky comment up to you this time," he says. Sorry Laks, I'm fresh out.

Google One member Daniel D. flexes "When you happen to be a Google One member, you will see this new top secret MIME type." Only for Google One members.

And speed demon Andreas C. flexes back, humble-bragging that 10Gb/s internet connectivity: "Gee, these powershell modules just gets larger and larger.."

1.3 Terabytes! Not too shabby.

Finally, remote worker Todd R. has uncovered the real root cause of WeWork's fall: poorly targeted direct mail campaigns. "There's a new Washington location, only 2,500 miles away. I'll stay here for now, thanks."

As this one might require a bit of orientation for our far-flung friends, here's a handy map from the US Library of Congress to explain US geography.

That's all for this week, but don't despair. The supply of Error'ds on the web is reliably non-decreasing.

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

https://thedailywtf.com/articles/the-other-washington

Oracle is… special. While their core product is their database software, what they actually sell is layers and layers of ERPs and HR systems that run on top of that database. And what they really make money on is the consulting required to keep those monsters from eating your company's IT team alive.

Because these ERPs are meant to be all things to all customers, you also will find that there are a lot of columns named things like attribute3. Your company's custom logic can stuff anything you want in there. "Do as thou wilt," as they say. And suffer the consequences.

For Steven H, his company had a requirement. The order lines needed to join to the manufactured batch that was used to fill that order. This makes sense, and is easy to implement- you add a join table that links BatchId and OrderLineId And, if the folks who build this feature did that, we wouldn't have an article.

To "solve" this problem, they simply mashed together all the order line IDs fulfilled by a batch into a text field called attribute7. The data looked like:

413314|413315|413329

That fulfilled the requirement, in someone's mind, and the ticket was closed and folks moved on to other work. And then a few years later, someone asked if they could actually display that data on a report. It seemed like a simple request, so it got kicked off to an offshore team.

This was their solution:

CREATE VIEW batch_order_lines_vw
AS SELECT bh.batch_id,
          ol.header_id,
          fields go here>
   FROM order_lines ol, batch_header bh
   WHERE 1=1
      AND ol.line_id IN ( SELECT TRIM( REGEXP_SUBSTR( bh.attribute7,
                                                      '[^|]+',
                                                      1,
                                                      LEVEL))
                          FROM DUAL
                          CONNECT BY REGEXP_SUBSTR( bh.attribute7,
                                                    '[^|]+',
                                                    1,
                                                    LEVEL)
                                            IS NOT NULL)
   ORDER BY line_id ASC

This query joins the batches to the order lines by using a REGEXP_SUBSTR to split those pipe-separated order lines. In fact, it needs to run the same regex twice to actually handle the split. In a subquery that is going to be executed for every combination of rows in order_lines and batch_header. Each table has millions of rows, so you already know exactly what this query does: it times out.

Speaking of things timing out, Steven has this to say about where this went:

We reported this to the database development team and marked the request as blocked. It's been maybe 2 years since then and it's still in that same state. I have since transferred to another team.

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

https://thedailywtf.com/articles/a-split-in-the-database

There are a lot of times when we want string comparisons to be case insensitive. It's quite a lot of cases, so every language is going to give us a way to easily specify that's what we want.

Take, for example, this C# code, written by one of Robin's team-mates.

override public void VerifyConnectionDetails(bool queryRequired)
{
    if (this.Name.EndsWith("2", StringComparison.OrdinalIgnoreCase) || this.Name.EndsWith("2", StringComparison.OrdinalIgnoreCase))
    {
        // Let some time pass to simulate PIDS behavior
        System.Threading.Thread.Sleep(100);
        IsConnected = false;
    }
    else
    {
        IsConnected = true;
    }
           
    IsConnected = true;
}

Here, we want to have two different code paths if the Name ends in "2". But we don't want one of those sneaky lower-case ₂s to throw things off, so we make this a case insensitive comparison.

Which, honestly, it's a perfectly reasonable thing to do. It may not have always been a "2" that they were looking for, so a case insensitive check may have made more sense in the past. But then… why do the check twice?

And this is where the flow of code drifts from "silly" to just weird. If we're on the "2" code path, we pause for 100ms and then set IsConnected to false. Otherwise, we set it to true. Then no matter what, we set it to true.

I suspect the "2" code path meant to have the false set before the sleep, to simulate checking a connection. Then it sets it to true, simulating that the connection has been established. But I don't know that from this code, instead what I see is a really weird way to force an IsConnected property to be true.

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

https://thedailywtf.com/articles/two-comparisons-hold-the-case

We all make simple mistakes. It's inevitable. "Pobody's nerfect," as they say, and we all have brain-farts, off days, and get caught up in a rush and make mistakes.

So we use tools to catch these mistakes. Whether it's automated testing or just checking what warnings the compiler spits out, we can have processes that catch our worst mistakes before they have any consequences.

Unless you're Jose's co-worker. This developer wasn't getting any warnings, they were getting compile errors. That didn't stop them from committing the code and pushing to a shared working branch. Fortunately, this didn't get much further down the pipeline, but said co-worker didn't really understand what the big deal was, and definitely didn't understand why there were errors in the first place.

In any case, here were the errors tossed out by the C# compiler:

Error: CS0165 - line 237 (593) - Use of unassigned local variable 'filter'
Error: CS0165 - line 246 (602) - Use of unassigned local variable 'filter'
Error: CS0165 - line 250 (606) - Use of unassigned local variable 'filter'
Error: CS0165 - line 241 (597) - Use of unassigned local variable 'filter'

Now, let's see if you can spot the cause:

    if (partnumber !="")
   {
       string filter="(PartPlant.MinimumQty<>0 OR PartPlant.MaximumQty<>0 OR PartPlant.SafetyQty<>0)";
   }
   else
   {
       string filter="PartPlant.PartNum = '" + partnumber + "'";
   }
         
   if (plantvalue !="")
   {
       string filter= filter + "";
   }
   else
   {
       string filter= filter + " AND PartPlant.Plant = '" + plantvalue + "'";
   }
  
   if (TPlantcmb.Text !="")
   {
       string filter= filter + "";
   }
   else
   {
       string filter= filter + " AND PartPlant.TransferPlant = '" + TPlantcmb.Text + "'";
   }

C#, like a lot of C-flavored languages, scopes variable declarations to blocks. So each string filter… creates a new variable called filter.

Of course, the co-worker's bad understanding of variable scope in C# isn't the real WTF. The real WTF is that this is clearly constructing SQL code via string concatenation, so say hello to injection attacks.

I suppose mastering the art of writing code that compiles needs to come before writing code that doesn't have gaping security vulnerabilities. After all, code that can't run can't be exploited either.

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

https://thedailywtf.com/articles/filtering-out-mistakes

"Don't use magic numbers," is a good rule for programming. But like any rule, you shouldn't blindly apply it. We know what happens when people do, however: we get constants that might as well be magic numbers.

Still, there are sometimes novel versions of this old song. Shmuel F sends us this one in C:

unsigned int ReadMemory(unsigned int address, char size)
{
   switch (size) 
   {
   case BIT3:
       // read byte-size
   case BIT5:
       // read int-size
   }
}

The cases of the switch statement are a clear threat- we have constants used that are just magic numbers. But the developer responsible went a little above and beyond in defining this:

#define ZERO    0
#define ONE     1
#define TWO     2
#define THREE   3
#define FOUR    4
#define FIVE    5
#define BIT0    (1 << ZERO)
#define BIT1    (1 << ONE)
#define BIT2    (1 << TWO)
#define BIT3    (1 << THREE)
#define BIT4    (1 << FOUR)
#define BIT5    (1 << FIVE)

Shmuel writes:

Seeing in the code ZERO and ONE is annoying. but this? this is just picking a fight.

All of this leaves us with one more question: why on Earth is size a bitmask?

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

https://thedailywtf.com/articles/are-you-doing-a-bit

While Error'd and TDWTF do have an international following, and this week's offerings are truly global, we are unavoidably mired in American traditions. Tomorrow, we begin the celebration of that most-revered of all such traditions: consumerist excess. In its honor, here are a half-dozen exemplary excesses or errors, curated from around the globe. They're not necessarily bugs, per se. Some are simply samples of that other great tradition: garbage in.

Opening from Poland, Michal reported recently of a small purchase that "The estimated arrival was October 27th. But, for a not-so-small additional fee, AliExpress offered to make an extra effort and deliver it as soon as... November 3rd."

Svelte Tim R. declines a purchase, reporting "ao.com had a good price on this LG laptop - the only thing that put me off was the weight"

Correct Kim accurately notes that "Getting the width of the annoying popup box is important if you want it to convey the proper message."

"As a leftie, I approve", applauds David H. "I certainly don't want any blots, scratches or muscle fatigue using this product!"

Ironically inflexible (says I, stretching hard for a gag) Melissa B. expresses dismay over an unexpected animal in her local library. "All I wanted was a yoga DVD. I wasn't expecting a surprise O'Reilly book on Programming Internet Email..." According to the reviews at Amazon, however, it apparently "contains interviews and stories of some of the biggest acts to ever get on stage such as KISS, Bon Jovi,Guns 'N Roses, Iron Maiden, Alice Cooper and many others." Who's to say there's not a yoga DVD tucked inside as well?

For the winning point, sportsman Philipp H. shares a gift search for his girlfriend, thinking "the smell of the beach might suit her more than the odor of football". I'm not sure of even that, Philipp. The beach isn't all coconuts, you know.

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

https://thedailywtf.com/articles/the-scent-of-a-woman