One of the big pieces of advice about using exceptions and errors is that they shouldn't be used for flow control. You catch an exception when an exceptional event happens, but you don't, for example, throw an exception to break out of a loop.

Unless you're Python, which throws exceptions to break out of loops. That's just how iteration works. But we're not here to talk about Python.

If a sufficiently motivated programmer can write LISP in any language, today's code, found by Maxi attempts to write Python in VB.Net. This snippet represents a general pattern around fetching data from temporary files.

DIM Buffer(5000) AS STRING
DIM Index AS INTEGER

ON ERROR RESUME NEXT

ERN=0
Index=0
WHILE ERN<>0
  IF(Buffer(Index)="") THEN ENDIF
  REM ...Do something...
  Index=Index+1
WEND

Here, we see the venerable VisualBasic On Error Resume Next. Before VB.Net, Visual Basic didn't have structured exception handling. You have a few options in pre-.NET Visual Basic for how to handle errors, but On Error Resume Next says "ignore errors, and I'll check to see if there's an error by examining the global variable Err.

Which, you'll note, Err is not ERN. Hidden behind the Do something comment, this code will perform an action which might fail, and set ERN = Err.Number- sometimes. It may also just set ERN itself, when it decides the loop is over. Sometimes this pattern is used to create retry blocks, where they only set ERN when the process succeeds.

Now, in this specific example, there's an additional factor. The Buffer is declared as an array of 5000 elements. So that seemingly do-nothing IF statement actually is vital: in ensures that once we cross 5000 iterations, it ensures that an error happens.

Every method uses On Error Resume Next, and pretty much every loop is written using this pattern. And honestly, that might not even be the worst of it. This is how they construct SQL statements, and what you're about to see is not anonymized or altered:

Sql="SELECT * FROM TableTemp WHERE xyz='"+Xyz+''"

Yes, the table name is actually TableTemp, yes the column is actually xyz, as in the SQL-injection-friendly variable.

Maxi is, slowly but surely, rolling a boulder up the hill, and promises that the code "is getting better". I fear that's a very tall hill.

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

https://thedailywtf.com/articles/on-error-goto-anywhere-but-here

Alex sends us some Objective-C code which… well, let's just say that the code is suspicious looking, but there's nothing clearly bad about it.

+(const char*)st:(NSArray*)a a:(int)i
{
    @try
    {
        return ([a objectAtIndex:i]==[NSNull null])?"":[[a objectAtIndex:i]UTF8String];
    }
    @catch (...)
    {
        return "";
    }
}

Alex had only this to say about the code: "What the hell is this even for?"

Now, this looks cryptic, but that's a mix of Objective-C and this developer's style. This defines a static/class method (the + symbol) which takes an NSArray and an integer parameter. Then, within a try/catch we fetch the objectAtIndex, with some null checking and a conversion to a UTF8String, returning a C-style string.

In actual use, this would be invoked like: [MyClass st:anArray a:5]. Which, aside from the terrible naming convention, this doesn't seem on its face bad. The try/catch gives us a usable behavior when i is out of bounds. It's a bad idea to use C-style strings in Objective-C (NSString would be preferred), but sometimes you need to.

So why is this code getting posted here? Let me re-post it with the comment describing its purpose:

//@try, @catch required as sometimes [a objectAtIndex:i] is an NSdictionary
+(const char*)st:(NSArray*)a:(int)i
{
    @try
    {
        return ([a objectAtIndex:i]==[NSNull null])?"":[[a objectAtIndex:i]UTF8String];
    }
    @catch (...)
    {
        return "";
    }
}

Most of the time, most of the entries in the array are NSStrings. But sometimes they're NSDictionarys. Why would you do this? Now, there's nothing in NSArray that prevents this- this isn't a setup with generics or templates, the NSArray is just an array of whatever you put in it. But please just put the same kinds of things in an array, for all our sakes.

[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/changing-your-type

Some time ago, Will started a new job- a four month contract to take an old, ugly DOS application and turn it into a new, shiny VisualBasic 6 application. "And," the new boss explained, "the new program is almost done anyway. It's just that the last guy left, so we need someone to take it over the finish line."

A key feature was that it needed to be able to fetch PDF files stored on a shared network drive. The order numbers were serialized, but the PDFs themselves were organized by year, creating file paths like I:\ORDLOG\2007\172.pdf. Now, in this particular block of code, one had access to both the ordnum and the ordyear, so constructing this path could be a trivial exercise. The previous developer did not take the trivial path, though.

    Select Case ordnum
            Case ordnum >= 18000
                 pdfstring = "I:\ORDLOG\2014\" + ordnum + ".pdf"
            Case ordnum <= 17999 And ordnum >= 16500
                 pdfstring = "I:\ORDLOG\2013\" + ordnum + ".pdf"
            Case ordnum <= 16499 And ordnum >= 15800
                 pdfstring = "I:\ORDLOG\2012\" + ordnum + ".pdf"
            ' ..........repeat for many lines to come
    End Select

So, the previous developer went through every year, identified the lowest and highest order number of that year, and then manually built a switch statement to decide with year that order number belonged to. Again, I want to stress, this block of code had access to an ordyear variable which contained the year information. Will replaced this entire statement with the more obvious solution.

But honestly, that's not the WTF. They're porting a DOS app to VB6 in 2014. The VB6 IDE hit end-of-life in 2008. While the VB6 runtime will be supported until the heat death of the universe, because Microsoft never breaks backwards compatibility, it's hard to develop code in a language where the only tool that can compile it hasn't been officially supported for six years. Speaking from experience supporting VB6 apps circa 2008, it was already difficult to get it actually working reliably on any relatively modern version of Windows aside from the most basic configurations. It got so bad that one of my main applications could only be built on one computer which had Windows Update disabled, because any attempt to do fresh installs of VB6 just failed outright.

VB6 and seeing 2014 in the code makes me hope that this story isn't from 2014, and instead they were future proofing the code and just predicting what orders they'd have in each year, because that's less awful, I think.

[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/select-orders

This submission left an anonymous reader speechless.

Handyman Luke H. hammers on an argument against visible test-in-prod. "More doing, less testing"

Student Mike S. shares a language lesson with us. " Obviously a new definition of OK."

Amateur meteorologist Loren Pechtel casts some shade on NOAA, reporting "from the weather station on Wheeler Peak, Nevada. The last column is snow depth." Seems like there's been less snow everywhere lately.

Connaisseur Gordon S. queries "Is DDG a big fan of the place, or is it telling me to go somewhere else?" Or is this a subtle SEO play?

Reader Valts S. volunteers: "Translation errors are kind of a low hanging fruit, and I even feel a bit guilty for submitting this. Obviously someone who's first language wasn't English tried hard and simply made a very human mistake. But then sometimes it happens to be so funny!" Too true, Valts.

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

https://thedailywtf.com/articles/it-s-funny-because-it-s-true

It's bad news when you inherit a legacy PHP CMS. It's worse news when you see that the vast majority of the files were last changed in 2002 (of course there's no source control).

That is the unfortunate position that Elda found herself in.

Like most CMSes, it needed to let users upload files. But we don't want to let users upload any arbitrary file, so we need to check that the file extension is correct. I mean, ideally, we want to check that the file is a valid file of the appropriate type so we don't serve up badfile.exe.mp3, but for an ancient PHP CMS just checking the extension is likely asking a lot. Especially when we see how this code performs that check.

function CheckFormat($extension)
{
        global $extensionPass;
        global $media_type;
        global $AllowedFormats;
        $extensionPass="false";
        if(($extension=="mp3")||($extension=="wma"))
        {
                $media_type="Audio";
                $extensionPass="true";
        }
        if(($extension=="wmv"))
        {
                $media_type="Video";
                $extensionPass="true";
        }
        $AllowedFormats="mp3, wma, wmv";
}

This is some impressively bad code. Instead of a function which returns a value, we have a trio of global variables that all get updated. Also, instead of using booleans, we get to use stringly typed true/false values.

This is also inside of a big, giant file. So we can just scroll down a ways and see how it's actually used:

// ...

$Mediafile_name=$_FILES['Mediafile']['name'];

// ...

$ext_array =explode(".",$Mediafile_name);
$last_position = count($ext_array) - 1  ;
$extension = $ext_array[$last_position] ;   //get photo extension
$extension=strtolower($extension);
if($extension=="") //couldn't get the extension which means probably the file was too large and so we ran out of memory
CheckFormat($extension);
if($extensionPass=="false")
{
        echo "
Incompatable Audio File
";
        echo "The file you uploaded cannot be used as it is in the format $extension.";
        echo "Allowed formats are $AllowedFormats.";
}
else
{
        // do something with the uploaded file
}

So, based on the comment couldn't get the extension, this tells us that they do no error checking at all on the $_FILES['Mediafile'] object. PHP offers methods like isset to check if a value exists, but no, instead of doing that, we'll chop up a string we might not have and only worry if that string turns out to be empty after chopping.

But, that comment is actually making the code harder to read. Let me remove that and fix the formatting so something becomes really, really obvious:

if($extension=="")
    CheckFormat($extension);

We only check to see if the extension is valid if it's empty. So we're 100% in the territory of "code that doesn't work". Since it doesn't even work, it's probably unfair to point out that the CheckFormat function sets a $media_type global to be "Audio" or "Video", and but the error message doesn't check that, meaning it outputs "Incompatable(sic) Audio File" even when you upload a video.

All that, and they also have a typo in the closing boldface tag inside that error message- . There's so much inside these small blocks that I think the // ... is doing a lot of covering for some other real ugly code.

[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/extensioning-yourself

Alexandar works with a veteran software architect. It's important to note here that a veteran is someone who has had experience. It certainly doesn't mean that they learned anything from that experience.

This veteran was given a task to write a C# method to populate a user's news feed. The goal was to find the five most recent news articles and add them to the list. Now, this is a large scale CMS, so those articles need to be fetched from ElasticSearch.

This was their solution:

[JsonIgnore]
public List AllNews
{
    get
    {
        var pages = SearchClient.Instance.Search()
            .ExcludeDeleted()
            .GetContentResult();

        if (this.NewsType == null)
        {
            return new List();
        }

        var returnedPages = pages.ToList().Where(_ => !string.IsNullOrEmpty(_.NewsType) && 
            _.NewsType.Contains(this.NewsType))
            .OrderByDescending(_ => _.PublicationDate).Take(5).ToList();

        return returnedPages;
    }
}

I'll let Alexandar summarize:

I find this snippet almost magical, every line is wrong, sometimes on multiple levels.

Alexandar overstates the wrong-ness- the lines that are just curly brackets or the word get are fine. It's all the other lines which are wrong.

First, let's start with the method name, AllNews. This does not fetch all news. It's supposed to fetch the five most recent stories. We're off to a good start.

But what about our fetch?

        var pages = SearchClient.Instance.Search()
            .ExcludeDeleted()
            .GetContentResult();

So, we search for all the news articles, excluding the deleted ones. There are a few problems with this. First, in addition to deleted articles, there are articles that are in draft states or articles which are embargoed until a certain date. Articles which we shouldn't show the users- which this method doesn't exclude.

There's also no ordering here, or a number of items we're requesting. Which means that, by default, this is going to return the first ten items it fetches. With no ordering, that means it's essentially ten random items- the first ten things ElasticSearch serves up.

So, once we have made a request and fetched ten random items which may or may not be appropriate to show our end users, we then check to see if there should be any news at all. If there's not, we return an empty page. We could have done that before sending a request to ElasticSearch, but who doesn't like throwing unnecessary requests into your processing pipeline?

Finally, we get to the LINQ calls:

        var returnedPages = pages.ToList().Where(_ => !string.IsNullOrEmpty(_.NewsType) && _.NewsType.Contains(this.NewsType))
            .OrderByDescending(_ => _.PublicationDate).Take(5).ToList();

So, we start with an unnecessary call to ToList because we like iterating over our results. That's fine, there's only ten of them. Then we have our Where which takes a lambda, and in this lambda we use the "discard operator" as our variable- the _ variable is meant to be used as a nonsense variable whose value we don't care about.

In the Where, we discard any entries that have no NewsType, whose NewsType doesn't contain the NewsType we're displaying. We started with ten random articles we might want to show the users, and after the filtering, we're down to… some number? Ten or less, depending on how lucky we are.

Finally, we sort by publication date, take the first five, and throw in one more ToList for good measure. I mean, the method signature says we're returning a List, so we need that ToList, because we certainly couldn't return one of the many other more abstract enumerable types that C# supports.

The end result is we have a method that returns between 0 and 5 randomly selected news articles, all sharing the same NewsType and sorted by PublicationDate. We call this AllNews.

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

https://thedailywtf.com/articles/all-the-news-you-need

Years ago, Aleshia W started a job in a VB.Net shop. There's a lot I could say about those kinds of environments, but I'd really just be padding out the article, so let's just get right to the code- which pads out a Year string.

Protected Function YearPadText(ByVal val As String) As String
    Dim valLen As Integer
    valLen = val.Len

    Select Case valLen
        Case 1
            val = val + "              "
        Case 2
            val = val + "             "
        Case 3
            val = val + "            "
        Case 4
            val = val + "           "
        Case 5
            val = val + "          "
        Case 6
            val = val + "         "
        Case 7
            val = val + "        "
        Case 8
            val = val + "       "
        Case 9
            val = val + "      "
        Case 10
            val = val + "     "
        Case 11
            val = val + "    "
        Case 12
            val = val + "   "
        Case 13
            val = val + "  "
        Case 14
            val = val + " "
    End Select
    Return val
End Function

You'll be unsurprised to learn that VB.Net has several different built-in ways of accomplishing this task, rendering this code unnecessary. One could complain that this code doesn't handle the situation when a year string is longer than 15 characters, but that raises a question: why would your year string be longer than 15 characters?

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

https://thedailywtf.com/articles/without-any-padding

Last week, we took a look at a hash array anti-pattern in JSON. This week, we get to see a Python version of that idea, with extra bonus quirks, from an anonymous submitter.

In this specific case, the code needed to handle CSV files. The order of the columns absolutely matters, and thus the developer needed to make sure that they always handled columns in the correct order. This led to code like this:

FIELD_NAME_ORDER = collections.OrderedDict({
  1: 'Field1',
  2: 'Field2',
  # etc. There are over a hundred fields.
})

# Elsewhere in the code, the only usage of FIELD_NAME_ORDER...
for field_name in FIELD_NAME_ORDER.values():
  AddField(field_name)

Now, the first thing you notice is that this is, once again, a hash array. The keys are the indexes. It doesn't look like that much of a WTF, and you'll note the use of OrderedDict which ensures that the dictionary retains insertion order. So this is just a silly little block of code…

Except, there are a few problems. First, starting around Python 3.7, OrderedDict became the default data structure for all dicts, so you don't really need the OrderedDict constructor in there. That's no big deal, except that prior to that version, a dictionary literal like {1: 'Field1', 2: 'Field2'} wouldn't be represented as an ordered dict- it would just be a hash, which means the order of the keys is arbitrary.

From the docs:

Keys and values are listed in an arbitrary order which is non-random, varies across Python implementations, and depends on the dictionary’s history of insertions and deletions.

Now, this code targets Python 2.7, which is old and out of support, and clearly TRWTF. But it 2.7, this absolutely was how dictionaries worked, so this code, on the surface, shouldn't work. But it does, and the reason isn't surprising once you think about it: what would you expect the unique hash of the number 1 to be?

CPython, the main implementation of Python, quite reasonably hashes ints to their value: hash(1) == 1. Non-OrderedDicts sort the keys in the order of their hash values. So the dict literal will iterate in the order of the numeric keys, and when we insert that into an OrderedDict it will preserve the insertion order, which is the numeric order.

The developer who wrote this blundered into a working solution by what appears to be an accident.

Our anonymous submitter took the extra few seconds to replace the OrderedDict with a list, which, y'know, is already going to guarantee order without you needing to blunder into how hashes work.

[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/ordering-the-hash

Music fan Erina leads off this week with a double contraction! "Who knew Tom Waits was such a gravelly-voiced Relational Database poet?" she Mused. "You'd've thought that SQL modes was more of an indy garage esthetic." You might've, Erina, but I wouldn't've.

An anonymous B2B buyer notes "It looks like the ERP software industry has anticipated all scenarios, even the restoration of the USSR." .ru serious.

Compulsive punster Dick Yates comments "I think Amazon is just feeding me a line."

Alphabetician David G. exclaims "Guess my company name wasn't long enough?" Maybe he needs to buy a few more vowels.

Finally, today's winner Simon A. shares an image that needs no added snark but that won't stop me. Says he "Your trial period expires in NaN days." You see, it's the undefined that has him fired him up. But for my part, I'm captured by that mental image of modern thumb restraints. Click, click, stay!

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

https://thedailywtf.com/articles/by-the-clicking-on-my-thumbs

Validating email addresses according to the actual email specification is more complicated than you usually think. Most homebrew validation tends to just get something that's relatively close, because hitting all the rules requires some fancy regex work. And honestly, for most applications, "pretty close to correct" is probably fine. If you actually care about collecting valid email addresses, you'll need to actually send mail to the address and have the user confirm receipt to "prove" that the email address is real, valid, and actually accessible.

Still, some "close enough" solutions are better than others. Jon found this C# code:

public bool EmailIsValid(String email)
{
    //Set defaults
    bool isValid = false;

    //Check email
    if (email.Contains("@") == true)
    {
        if (email.Contains(".") == true)
        {
            isValid = true;
        }
    }

    //Return
    return isValid;
}

In terms of overall style, this is one of those functions that seems written to hit all the things that annoy me to see in code, except nested ternaries. Checking if boolean functions == true. Nesting conditions. Adding intermediate variables that don't need to be there. The entire thing could be compressed into email.Contains("@") && email.Contains("."). It'd still be wrong, but at least it'd be readable.

In any case, while I tend to be forgiving of mildly incorrect email address validation, this one misses a lot of cases. A lot. And if you think it's not a WTF, then you can contact me at my new email address, @. and share your opinion.

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

https://thedailywtf.com/articles/where-you-at

Validation highlights the evolution of a programmer as they gain experience. A novice programmer, when given a validation problem, will tend to treat the string like an array or use substrings and attempt to verify that the input is the correct format. A more experienced programmer is going to break out the regexes. A very experienced programmer is going to just find a library or built-in method that does it, because there are better ways to use your time.

Andrea provides a rare example of a developer on the cusp between regexes and built-in methods.

public static bool isvalidIP(this string address)
{
    if (!Regex.IsMatch(address,
        @"\b\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}\b"))
        return false;

    IPAddress addr;
    return IPAddress.TryParse(address, out addr);
}

The first thing to note is that, based on the message signature, we know that this is a C# extension method. The static and this keywords tell us that. This means that this method can actually be invoked as if it were a member function of strings: "10.0.0.1".isvalidIP(). This is very much a code smell, in this case- unless the vast majority of strings this application works with are supposed to be IP addresses, it makes more sense to have a separate validation method. But that's me being picky, I suppose.

The first check done here is a regex check. In this case, if the string doesn't contain a word boundary, followed by 1-3 digits, followed by a dot, 1-3 digits, a dot, 1-3 digits a dot, 1-3 digits and finally another word boundary, it can't possibly be a valid IPv4 address. Which I don't think this is going to produce any false negatives, but it certainly produces some false positives: 999.999.999.999 is not a valid IP address, but passes the regex.

But that's okay, because they filter out the false positives by calling IPAddress.TryParse. This is the built in method that will take a string, and attempt to turn it into an IP address object. If it succeeds, it returns true (and stores the result in the out parameter). Otherwise, it returns false and stores a null in the out. This step makes the regex unnecessary.

Addendum: As pointed out in the comments, there's a deeper WTF, quoting from the docs:

For example, if ipString is "1", this method returns true even though "1" (or 0.0.0.1) is not a valid IP address and you might expect this method to return false. Fixing this bug would break existing apps, so the current behavior will not be changed.

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

https://thedailywtf.com/articles/validate-freely

There's a lot of room for disagreement in technology, but there's one universal, unchangeable truth: Oracle is the worst. But a second truth is that there's nothing so bad a programmer can't make it worse.

Someone at Ben's company needed to take data from a database and write it to a file. That file needed to have some specific formatting. So they used the best possible tool for the job: a PL/SQL stored procedure.

Now, PL/SQL is a… special language. The procedural elements it adds to SQL have a distinctly "we want to sell this to mainframe programmers" vibe, which makes the syntax verbose and clumsy. It frequently creates situations where things which should be easy are incredibly hard, and things which should be hard are impossible. But it's technically a feature-rich language, and you can even write web servers in it, if you want. And if you want to do that, you either work for Oracle or you should go work for Oracle, but certainly shouldn't be allowed out to mix with the general public.

In any case, Ben's predecessor decided to generate a carefully formatted text file in PL/SQL, and had… their own way of accomplishing things.

v_line := FIELD1 || '        ' || FIELD2 || '     ' || FIELD3 '          ' || FIELD4;
utl_file.put_line(write_file, v_line);
v_line := ' ';
utl_file.put_line(write_file, v_line);
utl_file.put_line(write_file, v_line);
utl_file.put_line(write_file, v_line);
utl_file.put_line(write_file, v_line);
utl_file.put_line(write_file, v_line);
utl_file.put_line(write_file, v_line);
utl_file.put_line(write_file, v_line);
utl_file.put_line(write_file, v_line);
utl_file.put_line(write_file, v_line);
utl_file.put_line(write_file, v_line);
utl_file.put_line(write_file, v_line);
utl_file.put_line(write_file, v_line);
utl_file.put_line(write_file, v_line);
utl_file.put_line(write_file, v_line);
v_line := …

Who needs a loop? Not this person. There's a hint, in this sample, though, that the entire thing is designed to be easily copy/pasteable. Every time they do output, they just dump utl_file.put_line(write_file, v_line) and just update v_line in between, pretty much guaranteeing that this'll be extra hard to debug when it eventually fails.

It's also worth noting that Ben supplied a small snippet, as a screenshot, which included line numbers. This block starts at line 163 of the procedure, and I suspect is followed by many, many more lines.

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

https://thedailywtf.com/articles/putting-the-file-out

When Arbuzo joined a new team, they helpfully provided him some sample code to show him how to interact with their JSON API. It was all pretty standard-looking stuff. If, for example, they fetched a Customer object, it would have some fields about the customer, and an array containing links to orders that customer had made. One of the samples helpfully showed iterating across the orders array:

let i = 1;
while(cust.orders[i]) {
    //do stuff with cust.orders[i]
    i++;
}

That got Arbuzo's attention, because it's such a weird and wrong way to solve this problem. Even if we ignore the arbitrary "start the array at 1" choice, it's such an awkward way to iterate across an array.

When Arbuzo checked the actual response data, however, he realized they weren't iterating across an array:

{
    "cust_id": 55,
    "cust_name": "Dewey, Cheatum, and Howe LTD",
    "cust_addr": …,
    …,
    "orders": {
        "1": "customer/55/orders/1",
        "2": "customer/55/orders/2",
        …
        "8": "customer/55/orders/12"
    }
}

There are cases where might want to have a map indexed by integers, like for example if you were making a sparse array. This is not one of those cases- all the order entries for each customer are simply incremented. I call this particular anti-pattern the "hash array": you're using a map to implement an array.

Abruzo couldn't rewrite the service, so he did the next best thing, and added a step to the response handling which did: cust.orders = Object.values(cust.orders) to turn things back into a proper array. Unfortunately, this wasn't the style laid out in the sample code Abruzo had been handed at the start, so it was rejected, and he also got to write lots of weird while constructs to traverse the hash arrays.

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

https://thedailywtf.com/articles/the-hash-array

This week's opening Error'd submission required a bit of translation for the monoglots among us, but it was worth the work. Not speaking even een beetje of Dutch, I was forced to use Google's own translation service to see what it was that had so worried our friend Sebas. And it's a doozy.

"...seek help - Child abuse images are illegal" warns Google's AI, inferring lewd Low Countries Linux links. For his part, Sebas takes it in stride, "Just hoping I'm not flagged now." I'm afraid to ask what the Goog makes of tcl.

Meanwhile, neighboring Daniel N. endured a slew of wtfery on the road to delivering us this archetypical Error'd. These are our raison d'^etre.
"Ironically, submitting directly to your website yields a 500 error!" he exclaims for WTF#1.
"Apologies for the fuzzy photo, I'm prohibited from taking screenshots on my work PC!" he continues for WTF#2. (But apparently not prohibited from car-shopping, eh Daniel? Don't worry, your secret is safe with us.)
Presenting below the WTF FTW, I give you:

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

https://thedailywtf.com/articles/innocents-abroad

Pete has had some terrible luck with the lead programmers he's worked with. He's had a few which are… well, they don't take feedback well. Like his current team lead, who absolutely doesn't let any of the other developers review or comment on his code. "Don't ask me questions, you should know this already," is a common refrain. Speaking of questions:

String q1 = form.getQ1()!=null?request.getParameter("question_" + form.getQ1().getId()):null;
String q2 = form.getQ2()!=null?request.getParameter("question_" + form.getQ2().getId()):null;
String q3 = form.getQ3()!=null?request.getParameter("question_" + form.getQ3().getId()):null;
String q4 = form.getQ4()!=null?request.getParameter("question_" + form.getQ4().getId()):null;
String q5 = form.getQ5()!=null?request.getParameter("question_" + form.getQ5().getId()):null;
String q6 = form.getQ6()!=null?request.getParameter("question_" + form.getQ6().getId()):null;
String q7 = form.getQ7()!=null?request.getParameter("question_" + form.getQ7().getId()):null;
String q8 = form.getQ8()!=null?request.getParameter("question_" + form.getQ8().getId()):null;
String q9 = form.getQ9()!=null?request.getParameter("question_" + form.getQ9().getId()):null;
String q10 = form.getQ10()!=null?request.getParameter("question_" + form.getQ10().getId()):null;
String q11 = form.getQ11()!=null?request.getParameter("question_" + form.getQ11().getId()):null;
String q12 = form.getQ12()!=null?request.getParameter("question_" + form.getQ12().getId()):null;                   
String q13 = form.getQ13()!=null?request.getParameter("question_" + form.getQ13().getId()):null;
String q14 = form.getQ14()!=null?request.getParameter("question_" + form.getQ14().getId()):null;
String q15 = form.getQ15()!=null?request.getParameter("question_" + form.getQ15().getId()):null;

Pete adds: "Note, this is only 15 lines of a file of his 2000 line file. This pattern is repeated for about 1900 lines. I guess he never learned about loops or arrays."

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

https://thedailywtf.com/articles/just-a-few-questions

Jan's company has an application which needs to handle an Excel spreadsheet, because as I'm fond of pointing out, users love spreadsheets.

The JavaScript code which handles parsing the spreadsheet contains… some choices. These choices caused it to fail on any spreadsheet with more than twenty six columns, and it's not hard to see why.

export function generateTableData(worksheet, headerRow) {
    const alphabet = ['A','B','C','D','E','F','G','H','I','J','K','L','M','N','O','P','Q','R','S','T','U','V','W','X','Y','Z'
    ];

    // first row is always 1 in the excel (in the loaded data array it is 0)
    let currentRow = parseInt(headerRow);//1;
    let cell = '';
    let tableData = [];
    let emptyRows = 0;

// Running until an empty line is found and breaks
    while (currentRow <= 10000) {
        let aIndex = 0;
        let newEntry = {};
        const cell_id = alphabet[aIndex] + currentRow;

// Running Excel columns from a-z. Needs adjustments for more columns!!!!!
        while(alphabet[aIndex] !== 'Z') {
            cell = worksheet[cell_id];
            const newIndex = aIndex + 1;
            if (cell !== undefined) {
                newEntry["col" + newIndex] = {value: cell.w, id: newIndex};
            } else {
                newEntry["col" + newIndex] = {value: EMPTY, id: newIndex};
            }
            aIndex += 1;
            cell_id = alphabet[aIndex] + currentRow;
        }

 // Run through every column of row and check for empty values
        let counter = 0;
        let emptyCounter = 0;
        for (const [key, value] of Object.entries(newEntry)) {
            counter += 1;
            if(value.value.indexOf(EMPTY) >=0) {
                emptyCounter += 1;
            }
        }
        // Row includes only empty values
        const isEmptyRow = emptyCounter == counter;
        if(isEmptyRow) {
            emptyRows += 1;
            if(emptyRows > 3) {
                break;
            }
        } else {
            emptyRows = 0;
        }

        if(!isEmptyRow) tableData.push(newEntry);
        currentRow += 1;
    }

    return [...tableData];
}

Now, I fully understand the choice of throwing together an alphabet array, from the perspective of just getting the code working. That's how Excel identifies its columns, and it's how this library expects you to access the columns. The problem here is that if your spreadsheet has more columns, we start doubling up- AA, AB, AC.

Which, while actually solving that problem that may have escaped the developer who implemented this, they had the good grace to call it out in a comment: Needs adjustments for more columns!!!!!

Of course, they don't actually use all 26 columns. The condition on their while loop stops when alphabet[aIndex] is equal to 'Z'.

But it's when we look at how they handle the contents of the Excel sheet that things get weird. We stuff each cell value into newEntry["col" + newIndex], which gives us an object with keys like col1, col2, etc. The end result is an array with extra steps and less useful index names.

After we've stuffed all those cells into the object with the awkward index names, we then iterate over that object again (without using those awkward index names) to count how many there are (despite knowing exactly how many times the while loop would have executed) and how many of those are empty- and that illustrates a lot about what "empty" is in this application:

value.value.indexOf(EMPTY) >=0

EMPTY is clearly a string constant. I don't know what it contains, but I certainly hope it's not a value that could ever appear in the spreadsheet, because if not, someday, somebody is going to put the word "EMPTY" in a cell in the sheet and confuse this code.

Finally, if we hit three empty rows in a row, we're clearly done. Or, if we hit the 10,000th row, we're done, just for bonus arbitrary magic numbers. Fortunately, nobody ever makes a spreadsheet with more rows than that.

All in all, this code reads like someone didn't fully understand the problem they were trying to solve, hacked at it until they got some messy thing that worked, committed it and called it a day. Since it worked, nobody looked at it until it stopped working.

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

https://thedailywtf.com/articles/a-parser-par-excellence

One of the arguments against comments in code is that they create a need to have two things updated: the code and the documentation have to be kept in sync. Inevitably, they'll drift apart.

David works with a junior developer who came onto the team with strong opinions about, well, everything. One of those strong opinions is that every single line needs to have comments. Each and every one.

This is the end result:

// update timeout status to 24rs
contact.ActivationValidUntil = providers.SystemDate.SystemDateTime.AddDays(30);

So, just a slight drift between the code and the comment. What's 29 days of drift between friends?

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

https://thedailywtf.com/articles/time-sensitive-comments

Folks who first learned to type on typewriters tend to prefer putting two spaces after a period.  Most of the rest of us prefer just one. And this may have caused a performance problem.

Rob's application had a quick search feature to track down customer claims. One day, the quick search was running quickly and efficiently. A user could type in a claim number, hit enter, and a moment later their screen would show the claim. Suddenly, it slowed down. It wasn't just the gradual decline of growing data or stale statistics or bad indexes. It was a code change, and it didn't take long to find the problem:

select claimid, masterclaimid, claimdata, claimtype 
from claim 
where upper(REGEXP_REPLACE(claimnum,'( ){2,}',' ')) = upper(:1 ) and claimtype in (0,1,2) 

Apparently, someone was typing two (or more) spaces someplace in the stored claim number. Rob has no information about why that was happening, and it's unlikely that some old-school typist was forcing extra spaces. I prefer to think that somebody in management had spilled a cocktail on their keyboard and now the spacebar was sticky, and spammed five or six spaces every time you pressed it once. Instead of getting their keyboard fixed, or the data in the database corrected, they had the code "fixed". Or maybe somebody just felt like their claim numbers should be whitespace insensitive, like HTML sort of is.

Without knowing why, we can still understand the bad idea. The REGEXP_REPLACE searches for ( ){2,}: a group containing a single space, repeated two or more times. Regexes are expensive at the best of times, regexes in the where clause of a query are going to make it much more expensive.

This change didn't go through any formal deployment process- someone with database credentials logged in, made the change, and didn't tell anyone. We don't know who, we don't know why, but what we do know is that Rob reverted it back to the version of the code in source control. No one complained.

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

https://thedailywtf.com/articles/a-little-extra-space

In the wake of yet another extraordinary ransomware attack, most businesses are finally beginning to implement the sorts of security measures they knew all along they should put in place. "Someday soon, when we get the time." Some writers have been calling it "unprecedented" but you and I know just how precedented it really is.

Loyal reader Aubrey leads off with an insider report, writing "Corporate IT recently migrated the entire company to a new antivirus program, and it seems to have flagged *its own update* as likely malware." That's what we call fastidious.

Infosec fan Peter G. comments "It's a 0day attack on a certificate!"

Matthew S. rhetorically wonders "How can I trust them to do my website when they can't even get their own encoding right?"

Andrew S. notes that many websites are not quite all-in on GDPR compliance. They might even be... reluctant. "When you don't really want to bother with GDPR consent, you can just forget to include the column."

Driving home the point that not everything this week is all about security or privacy, big spender Richard V. enthuses about his Very Generous Gift from Uber "WOW! What should I spend it on first?!"

Likely Mini driver Sam B. grouses "Incomprehensible API documentation? Eh, I've seen worse." We'd like to know where, so we can stay far far away.

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

https://thedailywtf.com/articles/malice-reflected

One of the "legacy" PHP applications needed a few bugfixes. "Legacy" in this case, means "written by a developer who doesn't work here anymore", so mostly everyone tried to dodge getting those bugfixes assigned to them. Joe was taking a three day weekend at the time, so a helpful co-worker assigned the tickets to him.

The code wasn't an absolute disaster, but it suffered from being written by a "smart" programmer. Since they were so smart, they couldn't just do things using the basic language constructs, they had to find clever ways to abuse them.

For example, why would you write a for loop, with a counting variable, when you could do this instead?

$a = array_fill(0, 100, 1);
foreach($a as $i)
{
  //whatever
}

Yes, they populate an array with the range of their loop, and then just foreach over the array. This pattern was standard for this developer, nary was a traditional for loop to be found.

Now sometimes, this developer also needed to know which iteration they were on, so sometimes they would capture the array index, like:

$a = array_fill(0, 100, 1);
foreach($a as $i => $one)
{
  //whatever with $i
}

This is basically the same as above, except the key (or index) of the element in the array gets stored in $i, while the value gets stored in $one. Now, if you're thinking to yourself that array_fill means that there's a direct relationship between these two values, so that's completely unnecessary, you win a prize. The prize is that you don't have to work on this code.

As an aside, while I was checking the specific PHP syntax for foreach, I found this caveat to manipulating arrays by reference, which cautions about some absolutely bizarre side-effects. That has nothing to do with this code, but just demonstrates another way in which PHP can harm you.

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

https://thedailywtf.com/articles/another-iteration