On a vacation trip this week. The diligent will be able to discover the location with a tiny bit of sleuthing, but I won't be there now.

An anonymous traveler reports "I've been trying to contact them but I don't think they check their email very often."

 

Naturalist Mike has a bone to pick with the Brevard Zoo, subtly suggesting` "I'm not sure this conservation message is on point."

 

Faithful Michael R. beefs with LinkedIn's date grammar: "Fast forward into the past, LinkedIn."

 

Hardcore Daniel hammers Manhattan. "I didn’t know it, but apparently I enjoy biking up and down Broadway faster than Lance Armstrong."

 

Finally, another anonymous player wonders "Does this mean I will learn 8 minutes in 3 minutes or 3 minutes in 8 minutes?" It means Lean Six is really only Two, obviously.

 

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

The addition of nullable or optional types to mainstream languages was a net good. It doesn't completely solve the billion dollar mistake, but it makes it far easier to write safe code.

For most of us anyway.

Sam found this representative line, which shows how one of his peers understand nullable types to work:

DateTime? current = new DateTime?();

I actually don't think I've ever seen anyone create an instance of the nullable wrapper directly, like this. I've never contemplated doing it. The more traditional usage would be something like:

DateTime? current = someFunctionWhichMayReturnAValueOrNull();

We don't know if we got a null or not, but because it's wrapped in a nullable type, we can still handle it safely without risking a null reference exception.

Instantiating a nullable type directly results in a nullable type that is known to be empty. Which I can imagine some uses for, I suppose, but still seems like a real weird choice. And it's unclear- if you really wanted that, you'd just do DateTime? current = null; which is a more obvious way to say the same thing.

In the end, I'm not certain this is actually a WTF, but it still perplexes me. And it's a representative line- this pattern appears everywhere in Sam's codebase, with enough frequency that it's more of a surprise when people use nullables the standard way.

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

Peter was tracking down some bugs, when he found code which looks like this:

if (IsEmptyOrNull(myInput)) { // do things that clearly expect myInput to NOT be null or empty } else { throw BadInputException("The input must not be null."); }

Names are made up above, to illustrate the flow of code.

This seemed wildly wrong, and was possibly the source of the bug, so Peter dove in. Unfortunately, this wasn't the bug. You see, IsEmptyOrNull is not a built-in function. But it wraps one.

public bool IsEmptyOrNull(string param1) { return !String.IsNullOrEmpty(param1); }

Wrapping a small built-in function is already a code smell. Making the name almost identical but not quite is also a code smell. But reversing the meaning because you reversed the name is absolutely bonkers.

Did they think that A or B != B or A? Because that's what this implies. The fact that anyone used this function, when its usage was so clearly contradicting its name, speaks to a deep level of nobody caring.

It was, at least, an easy refactoring. But it speaks to how thoroughly broken their codebase is.

.comment { border: none; } [Advertisement] Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!

Lisa was working on a project she quite liked. The tech was cool, the problems being solved were interesting, and the team had a good working process. Company-wide, however, budgets were tight, and other projects were in much worse shape, so the project Lisa was on got put on pause, and her team was moved onto a different project.

Someone wanted to make sure that functions which had large side effects were only called in the right places. Now, most of us might use some mixture of public/private, clear documentation, and maybe some key flags and error checking to ensure this was the case.

This team had a… different approach.

// This is called so that Foo will unload all widgets before exiting. // It is currently only called from Form1.Closing(). A password is *required*. If not correct, this function immediately returns. public void UnloadAll(string pwd) { if (pwd == "FOO-> UNLOAD ALL") { ProcessRequest(RequestType.Unload, Environments.All); } }

The caller must supply a password to this method, otherwise it does nothing. I want to stress, this isn't a password we expect the user to type in (having that hard-coded in the application code is a different WTF), but instead is a token that the calling code must supply if they want the function to execute.

This entire project exists in a single .NET Assembly, and the keyword private is never used once.

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

Today's anonymous submitter, who we'll call Sally, works on medical devices. As you can imagine, the development process for such devices is very strict. I mean, it should be, but we know that the industry has problems.

Unfortunately for Sally, one of those problems is the tech lead on a project she is inheriting. Said tech lead is leaving, and Sally is coming on to replace them. The project is in C#, and Sally is the most experience with the language, making her the obvious choice to step in.

Now, the current tech lead had some concerns about the development cycle. You see, the whole process of writing code, compiling code, deploying that code onto hardware, and then running the code just took too darn long. If you wanted to iterate as fast as possible, you needed to skip some of those steps.

internal static Action<InstrumentState> Compile(IEnumerable<Configuration.Rule> rules) { var code = string.Format(@" using System; using SomeCompany.SomeProject.Instrument; using SomeCompany.SomeProject.Instrument.State.Actions; using ConsoleState = StateMachine.Types.State; namespace SomeCompany.SomeProject.Instrument.State.Reducers {{ public class Script {{ private static bool _done; private static void Done() {{ _done = true; }} public static void Execute(InstrumentState state) {{ _done = false; {0} }} {1} }} }} " , string.Join(Environment.NewLine, rules.Select((i, o) => string.Format(@" if (!_done) {{ rule{0}(state); }} ", o))) , string.Join(Environment.NewLine, rules.Select((i, o) => string.Format(@" private static void rule{0}(InstrumentState state) {{ if ({1}) {{ {2} }} }}", o, i.Trigger, string.Join(Environment.NewLine, i.Actions)))) ); var types = new[] { typeof(Console), typeof(InstrumentState), typeof(ErrorEventAction), typeof(ComponentId), typeof(global::StateMachine.Types.State) }; var refs = types.Select(h => MetadataReference.CreateFromFile(h.Assembly.Location) as MetadataReference).ToList(); //some default refeerences refs.Add(MetadataReference.CreateFromFile(Path.Combine(Path.GetDirectoryName(typeof(System.Runtime.GCSettings).GetTypeInfo().Assembly.Location), "System.Runtime.dll"))); refs.Add(MetadataReference.CreateFromFile(typeof(Object).Assembly.Location)); var parse = CSharpSyntaxTree.ParseText(code); var assyName = Guid.NewGuid().ToString(); var options = new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary, allowUnsafe: true, optimizationLevel: OptimizationLevel.Release); var compilation = CSharpCompilation.Create(assyName, new List<SyntaxTree> { parse }, refs, options); var state = Expression.Parameter(typeof(InstrumentState), "state"); Action<InstrumentState> y = (_) => { }; using (var stream = new MemoryStream()) { var result = compilation.Emit(stream); if (!result.Success) { var compilationErrors = result.Diagnostics.Where(diagnostic => diagnostic.IsWarningAsError || diagnostic.Severity == DiagnosticSeverity.Error) .ToList(); if (compilationErrors.Any()) { var firstError = compilationErrors.First(); var errorNumber = firstError.Id; var errorDescription = firstError.GetMessage(); var firstErrorMessage = $"{errorNumber}: {errorDescription};"; var exception = new Exception($"Compilation failed, first error is: {firstErrorMessage}"); compilationErrors.ForEach(e => { if (!exception.Data.Contains(e.Id)) exception.Data.Add(e.Id, e.GetMessage()); }); throw exception; } } else { stream.Seek(0, SeekOrigin.Begin); var assy = AssemblyLoadContext.Default.LoadFromStream(stream); var type = assy.GetType("SomeCompany.SomeProject.Instrument.State.Reducers.Script"); y = Expression.Lambda<Action<InstrumentState>>(Expression.Call(type.GetMethod("Execute"), state), state).Compile(); } } return y; }

You know when the first line creates a variable code and it holds C# code, you're in for a time.

The first block of code here does a lot. It queries the rules object- a reference to our settings database- to generate a series of rule{0} functions, where the {0} is some name. The body of the function has a condition on i.Trigger (ensuring this rule only applies sometimes) and then has a body defined by i.Actions, which is just C# code living in our data source.

We then populate an Execute function with calls to every rule{0} function we generated. These themselves are further gated by a _done check, meaning it's possible for some rules to abort the processing of further rules.

The rest of the code here is a set of interactions with the C# compiler. We parse and compile the C# code that we just munged together through string concatenation, emit that compiled data into an in-memory stream, and if it succeeds in compilation, we create a C# assembly based off that stream. That is to say, we generate and execute a library without leaving anything on the filesystem for further review. It all exists purely in memory.

We then generate a lambda which calls the Execute function in that newly generate library, and return it, so that other callers can now use it.

There are so many things wrong with this. Setting aside the code generation, the code that gets generated is complicated: a chain of statements where each has its own dedicated trigger condition and may be skipped based on a done flag. Just trying to analyze that for any non-trivial set of rules is hard.

The code generation, however, is the real WTF. First, they were using a set of static analysis tools to try and maximize code safety. None of the code living in the settings database went through that. Second, the settings database was editable by customers. Doctors were expected to edit it. The very idea that you could change the code running on the device by editing the Settings database was a huge "No!" But the bonus of doing this all in memory means that if there were a breach, it'd be trivially easy for an attacker to cover their tracks.

Now, there was no malice on the part of the tech lead. This was all just for personal convenience to speed up iterating on the code. It wasn't an intentional security flaw- it was just clever.

Sally raised this to her boss. He sighed, put his head in his hands, and was silent for a long moment. "We just signed off on doing pen-testing last week. They're going to destroy us."

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

Elte Hupkes "Some weird Android behavior has my phone disconnecting from WiFi until I open it back up in the morning, triggering some backups. Unfortunately, WhatsApp Backup isn't a morning person."

 

"User Permissions - Small Town Midwest Style" is how Jeremy proposed to title this submission, explaining "This how one particular school is set up in our district's library management system. I guess this makes it easier when a school secretary puts in a ticket saying: Janet is our new office assistant - please give her the same access that Barb had."

 

Confused Mark W. exclaimed "QR codes have sure changed!" That's the new combination QR-Captcha.

 

Quoth Tyler "While filling out some forms for an appointment, I had to select a language. I wonder if they'd actually have a translator available for old or middle English! The best worst part is, the appointment was for my newborn son and neither 'none', 'baby', nor 'gibberish' were alternative options."

 

Finally, The Beast in Black is back with a love letter from Google's lonelyhearts AI, reporting "Gmail's Smart Reply AI seems to REALLY like LinkedIn recruiters. Either that, or their LLM needs a teeny bit more work."

 

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

Today's anonymous submitter found some Java code which finds the largest value in a quartet of floats. Now, the code is quite old, so it actually predates varargs in Java. That doesn't excuse any of what you're about to see.

public float CalculateMaximumValue(float a, float b, float c, float d) { int i = 0; float[] arr = new float[] { 0, 0, 0, 0 }; float gtval = 0; for (i = 0; i < 4; i++) { arr[i] = 0; } arr[0] = a; arr[1] = b; arr[2] = c; arr[3] = d; gtval = arr[0]; for (i = 0; i < 4; i++) { if (arr[i] > gtval) { gtval = arr[i]; } } return gtval; }

The best thing I can say about this is that they didn't use some tortured expansion of every possible comparison:

if (a > b && a > c && a > d) return a; if (b > a && b > c && b > d) return b; …

Honestly, that would be awful, but I'd prefer it. This just makes my eyes sting when I look at it.

But let's trace through it, because each step is dumb.

We start by creating an empty array, where every value is initialized to zero. This isn't necessary, as that's what Java does by default. But then, we loop across the array to set things to zero one more time, just to be sure.

Once we're convinced every value is definitely zero, we replace those zeroes with the real values. Then we can loop across the array and find the largest value with straightforward comparisons.

This code is, in some ways, the worst kind of code. It's bad, but not so bad as it's ever going to cause real, serious problems. No one is going to see any bugs or inefficiencies coming from this method. It's just an ugly mess that's going to sit there in that codebase until the entire thing gets junked, someday. It's just an irritant that never rises to the level of frustration which drives action.

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

Kendall found some recently written code, and had to wonder, "Who wrote this crap?" Much to Kendall's disappointment, source control knew who wrote it: Kendall.

if (x < 0.0) { x += 0.0 - x; width -= 0.0 - x; }

Kendall didn't share the purpose of this code, but based on starting with a less-than-zero check, I suspect the goal was to do something akin to an absolute value. If x is less than zero, make it positive.

That's certainly what was attempted. 0.0 - x, where x < 0 would be the same as x * -1. Unfortunately, Kendall added that to x, making x zero.

As with a disappointingly large quantity of bad code, this got committed without any tests, rolled out to production, and created head-scratching bugs for months. Eventually, the bugs became annoying enough that they bubbled up to the top of the priority list, and Kendall was tasked with fixing them.

The other reason I think the goal was essentially an absolute value operation is Kendall's commentary:

Aside from the major bug, this code is a sure indicator of overthinking things.

It is an overly complex way to flip the sign, yes. But "overthinking?"

The line between overthinking and underthinking is a thin line indeed.

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

Sebastian started a new job recently. Like a lot of "I started a new job," stories, this one starts with a 1,000 line class definition. What's notable about this one, however, is that most of that code is error handling. Now, you might think to yourself, "well, there's nothing inherently wrong with loads of error handling, if the problem calls for it.

This code is getting posted here. Do you think the problem calls for it?

object Method1(MyOtherClass parameter) { try { if(parameter == null) throw new ArgumentNullException(); //... 5 lines of code } #region catching catch(FormatException) { return null; } catch(InvalidOperationException) { return null; } catch(Exception) { return null; } #endregion } bool Method2(MyOtherClass parameter) { try { result = Method1(parameter); if(result == null) throw new Exception(); // ... 3 lines of code } catch(Exception) { return false; } }

Names have been anonymized by Sebastian.

We open with a mostly reasonable bit of code- if the input parameter violates our contract, we throw an exception. I don't love exceptions for communicating contract violations- it's much better if you can enforce that at compile time- but I won't fault anyone for that. But gee, isn't it a bit odd that we throw that exception inside of a try block?

Oh, that's because we catch the exceptions and return null. The fact that we catch multiple kinds of exceptions and just return null is already bad. It gets worse when we note that the last caught exception is Exception, the root of all exception classes.

Normally, when we talk about the anti-pattern of using exceptions for flow control, we tend to think of them as spicy gotos, which is exactly what's happening here. It's just… we've removed the spice. It's Minnesota Spicy Gotos- where the three grains of black pepper are too much zing. We're jumping to the appropriate return statement. Which we could have just replaced the throw with a return. And you can't even say that they're trying to follow the rule of "only have one return".

The calling function makes the whole pattern worse. We invoke Method1, and if it returns null (that is to say, if it throws and catches its own exception), we… throw another exception. Which leads us to a return false.

Sebastian tells us that this 1kloc class is about 70% error handling code, by volume, and as we can see, none of these errors need to happen.

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

When discussing ternaries, we also have to discuss readability. While short and concise, they're in some ways too compact. But don't worry, Mark's co-worker has a wonderful simplification to ternaries. This representative line is a pattern used throughout the codebase.

pnlErrorMessage.Visible = !string.IsNullOrEmpty(errorMsg) ? true : false;

This is genius, as the ternary becomes documentation for a boolean expression, telling us when we're setting things to true or false without having to think about what the expression we're evaluating means. If there is an error message, we set the error message UI element's visibility to true. Explicit, verbose, and readable.

What we're really looking at here is the ol':

if (expression) return true; else return false;

pattern, compressed into a single ternary. Annoying, useless, and a hint that our developer doesn't understand booleans.

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

"Grocery stores are going too far with their energy foods" charged Tim DG. "I was just looking for some salads to go with my BBQ," he complained. "I'm not sure they sell what I'm looking for." I've seen what your kin put in their Huzarensaladen, Tim, so I'm not entirely surprised about the Duracells.

 

Long-suffering Gordon S. found a novel Error'd, at least, I don't remember having seen this before. "Left Spotify running and came back 15 minutes in on a 3 minute song. Is this how extended play records worked?" I'm glad he only submitted it once and not a hundred more times for art's sake.

 

Christopher P. thinks FedEx is on the verge of building robots with Genuine People Personalities. "It appears to be impossible to contact a human at FedEx, and their bot seems very passive aggressive when I gave it a negative rating it tries to cancel my case. Fantastic. " I'm sure it's not truly impossible, only very very improbable.

 

Experienced Drinker Peter G. thinks this is not really an Error but it's a little bit of a WTF. "This is the gatekeeper popup that blocks your way when you visit the Quantum Spirits web site, which for some reason has decided to limits its customer base to a very narrow demographic. No, I'm not 21, and haven't been for quite some time." People should say what they mean and not place the burden of decoding their imprecision on everyone else.

 

Michael Th. is making me hungry. "Had a lovely dinner in a nice restaurant in Mannheim, Germany - and the service was really SUperb!" Once again, not really an Error'd but a sign that somebody is using bad practices with their POS system.

 

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

We all know that building SQL queries via string concatenation, and then sending them to the database, is just begging for fragile code and SQL injection attacks. But, what if the bad part is the "sending them to the database" part? Has anyone ever thought about that?

Kris's predecessor has.

CREATE PROCEDURE [dbo].[usp_LossMit_GetCDCMappingInfo] @PropertyNameString NVARCHAR(4000), @Environment CHAR(1) AS BEGIN DECLARE @TICK CHAR (1) SET @TICK = CHAR(39) DECLARE @SQLSelect NVARCHAR (4000) DECLARE @SQLWHERE NVARCHAR (4000) DECLARE @SQLSelectII NVARCHAR (4000) DECLARE @SQLWHEREII NVARCHAR (4000) SET @SQLSelect = ' SELECT CDCID As PropertyValue, CDCName AS EntityName, ISNULL(RTRIM(PropertyName), '+ @TICK + @TICK + ') AS PropertyName FROM dbo.LossMitCDCIDMapping' SET @SQLWHERE = ' WHERE PropertyName IN (' + @PropertyNameString + ') AND Environment = ' + @TICK + @Environment + @TICK + 'AND IsActive = 1' SET @SQLSelectII = ' UNION SELECT lccm.CDCControlID AS PropertyValue, lccm.CDCControlName AS EntityName, ISNULL(RTRIM(lccm.PropertyName), '+ @TICK + @TICK + ') AS PropertyName FROM dbo.LossMitCDCIDMapping lcm INNER JOIN dbo.LossMitCDCControlIDMapping lccm ON lcm.CDCID = lccm.CDCID' SET @SQLWHEREII = ' AND lcm.PropertyName IN ( '+ @PropertyNameString + ') AND lcm.Environment = ' + @TICK + @Environment + @TICK + ' AND lccm.Environment = ' + @TICK + @Environment + @TICK + ' AND lcm.IsActive = 1 AND lccm.IsActive = 1' PRINT (@SQLSelect + @SQLWHERE + @SQLSelectII + @SQLWHEREII) EXEC (@SQLSelect + @SQLWHERE + @SQLSelectII + @SQLWHEREII) END /*****usp_LossMit_GetAutoIndex******/ GO

Now, just one little, itsy-bitsy thing about T-SQL: it handles variables in SQL statements just fine. They could have written AND Environment = @Environment without wrapping it up in string concatenation. This entire function could have been written without a single string concatenation in it, and the code would be simpler and easier to read, and not be begging for SQL injection attacks.

And I have no idea what's going on with @TICK- it's a one character string that they set equal to an empty 39 character string, so I assume it's just ""- why are we spamming it everywhere?

And not to be the person that harps on capitalization, but why @SQLSelect and @SQLWHERE? It's next-level inconsistency.

My only hypothesis is that this code was originally in ASP or something similar, and someone said, "Performance is bad, we should turn it into a stored procedure," and so someone did- without changing one iota about how the code was structured or worked.

Kris has this to say:

Just started at a new job--it's going to be interesting…

Interesting is certainly one word for it.

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

Many years ago, Sam was obeying Remy's Law of Requirements Gathering ("No matter what your requirements actually say, what your users want is Excel") and was working on a web-based spreadsheet and form application.

The code is not good, and involves a great deal of reinvented wheels. It is, for example, Java based, but instead of using any of the standard Java web containers for hosting their code, they wrote their own. It's like Java Servlets, but also is utterly unlike them in important and surprising ways. It supports JSP for views, but also has just enough surprises that it breaks new developers.

But let's just look at how it handles form data:

// form field information String[] MM_fields = null, MM_columns = null; // ...snip... String MM_fieldsStr = "phone|value|organization|value|last_name|value|first_name|value|password|value|email_opt_in|value"; String MM_columnsStr = "phone|',none,''|organization|',none,''|last_name|',none,''|first_name|',none,''|password|',none,''|email_opt_in|none,1,0"; // create the MM_fields and MM_columns arrays java.util.StringTokenizer tokens = new java.util.StringTokenizer( MM_fieldsStr, "|" ); MM_fields = new String[ tokens.countTokens() ]; for (int i=0; tokens.hasMoreTokens(); i++) MM_fields[i] = tokens.nextToken(); tokens = new java.util.StringTokenizer( MM_columnsStr, "|" ); MM_columns = new String[ tokens.countTokens() ]; for (int i=0; tokens.hasMoreTokens(); i++) MM_columns[i] = tokens.nextToken();

Who doesn't love hard-coded lists of strings with characters separating them, which then need to be parsed so that you can convert that into an array?

The MM_fieldsStr seems to imply the input data will be "key|value" pairs, and the MM_columnsStr seems to imply a specific default value, I think- but look at those quotes and commas. This is generating strings which will be injected into JavaScript. And who knows what's happening on that side- I certainly don't want to.

Also, what even is the MM_ prefix on our variables? It looks like Hungarian notation, but conveys no information- maybe it's Rēkohu notation?

As you can imagine, this whole solution was incredibly fragile and didn't work well.

.comment { border: none; } [Advertisement] Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!

One of the first things anyone learns about object oriented programming is the power of inheritance and overriding functions. Isn't it great that you can extend or modify the implementation of a function in your derived classes? Don't you wish you could just do that for every function? Aash's co-worker certainly does.

@Override public boolean isEnabled() { if (!super.isEnabled()) { return false; } return true; }

I think this is a beautiful little smear of bad code, because it's useless on multiple levels. First, we are calling a boolean function only to bury it in a conditional which does the exact same thing: return super.isEnabled() would do the job. But if our developer thought to do that, they'd instantly see that there's no point to adding an override- we're just doing what the super class does. The if is just enough to hide that from you if you're careless and not very bright.

And, before you ask, no, there never was any real functionality in this override, at least not that ever got checked into source control. It isn't a vestigial leftover of once useful code. It's just useless from birth.

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

One of the clients for Rudolf's company was getting furious with them. The dev team was in constant firefighting mode. No new features ever shipped, because the code-base was too fragile to add new features to without breaking something else. What few tests existed were broken. Anyone put on the project burned out and fled in months, sometimes weeks, and rarely after only a few days.

Rudolf wasn't too pleased when management parachuted him into the project to save it. But when he pulled the code and started poking around, it looked bad but not unsalvageable. The first thing he noticed is that, when following the instructions in the README, he couldn't build and run the application. Or maybe he wasn't following the instructions in the README, because the README was a confusing and incoherent mess, which included snippets from unresolved merges. Rudolf's first few days on the project were spent just getting it building and running locally, then updating the README. Once that was done, he started in on fixing the broken tests. There was a lot of work to be done, but it was all doable work. Rudolf could lay out a plan of how to get the project back on track and start delivering new features.

It's about then that Steve, the product owner, called Rudolf in to his office. "What the hell do you think you're doing?"

Rudolf blinked. "Um… what I was asked to do?"

"Three days and you just commit a README update? A couple of unit tests?"

"Well, it was out of date and meant I couldn't-"

"Our client is crazy about their business," Steve said. "Not about READMEs. Not about unit tests. None of that actually helps their business."

Rudolf bit back a "well, actually," while Steve ranted.

"Next thing you're going to tell me is that we should waste time on refactoring, like everybody else did. Time is money, time is new features, and new features are money!"

Suddenly, Rudolf realized that the reason the project had such a high burnout rate had nothing to do with the code itself. And while Rudolf could fix the code, he couldn't fix Steve. So, he did what everyone else had done: kept his head down and struggled through for a few months, and kept poking his manager to get him onto another project. In the meantime, he made this code slightly better for the next person, despite Steve's ranting. Rudolf eventually moved on, and Steve told everyone he was the worst developer that had ever touched the project.

The customer continued to be unhappy.

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

Daniel D. humblebrags that he can spell. "Ordering is easy, but alphabet is hard. Anyway for this developer it was. Can anyone spot which sorting algo they used?" Next he'll probably rub it in that he can actually read unlike the TDWTF staff. I guess we'll never know.

 

"We're all artsy in Massachusetts," explains Bruce C. as some kind of justification for this WTF. "My university is validating its records. Now I see why they have problems."

 

"Who knew they were twins?" tittered topical Walt T. "Only one twin can be VP at a time!" Or as the case may be, neither.

 

Never Forget the 11th of Septiembre, observes Tim K. "I've been meaning to submit a WTF in the past few days but this came upon my doorstep this morning."

 

"An uppercase 5?", Greg grumbled. "I know JavaScript is hard, but I guess Kroger's backend system for gift cards can only handle uppercase numbers."
It should come as no surprise to anyone who has been hanging around these parts for any length of time, but for every rule you take for granted, there is inevitably an exception. In this case, there are two. Exception the frist: 5 is already upper case. The thing that you're missing is actually a lower-cased 5. Exception the snecond: Chinese does have both "big" (dàxiě) and "little" (xiǎoxiě) forms of its number representations.

 

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

There's nothing inherently wrong with the ternary operator. It's just the kind of thing that gets abused.

Now, we all know how it should be used. We frequently would write something like this:

let val = arr.length>0?arr[0].id:0;

If the array contains elements, grab the first one, otherwise use a reasonable default. It's not my favorite convention, but it's fine. Nothing worth complaining about.

Lahar Shah's co-worker has a different approach to this.

// Set value for tempVariable arr.length > 0 ? tempVariable = arr[0].id : tempVariable = null;

It's amazing how converting a ternary from an expression which evaluates to a value into a statement which changes program state makes it feel so much grosser. There's nothing technically wrong with this, but it makes me want to set the code on fire and dance naked around the flames.

This, of course, wasn't a one-off use of the ternary operator. This was how the developer used the ternary, forever and always.

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

"Dark Horse" inherited some PHP code. They had a hundred lines to submit, but only sent in a dozen- which is fine, as the dozen lines tell us what the other hundred look like.

$suite_1_1 = number_format($item -> {'suite_1_1_'.$the_currency}, 2, '.', ''); $suite_1_2 = number_format($item -> {'suite_1_2_'.$the_currency}, 2, '.', ''); $suite_1_3 = number_format($item -> {'suite_1_3_'.$the_currency}, 2, '.', ''); $suite_1_4 = number_format($item -> {'suite_1_4_'.$the_currency}, 2, '.', ''); $suite_2_1 = number_format($item -> {'suite_2_1_'.$the_currency}, 2, '.', ''); $suite_2_2 = number_format($item -> {'suite_2_2_'.$the_currency}, 2, '.', ''); $suite_2_3 = number_format($item -> {'suite_2_3_'.$the_currency}, 2, '.', ''); $suite_2_4 = number_format($item -> {'suite_2_4_'.$the_currency}, 2, '.', ''); $suite_3_1 = number_format($item -> {'suite_3_1_'.$the_currency}, 2, '.', ''); $suite_3_2 = number_format($item -> {'suite_3_2_'.$the_currency}, 2, '.', ''); $suite_3_3 = number_format($item -> {'suite_3_3_'.$the_currency}, 2, '.', ''); $suite_3_4 = number_format($item -> {'suite_3_4_'.$the_currency}, 2, '.', '');

On one level, they have an object called $item, and want to format a series of fields to two decimal places. Their approach to doing this is to just… write a line of code for each one. But this code is so much worse than that.

Let's start with the object, which has fields named in a pattern, suite_1_1_USD, and suite_2_1_EUR. Which right off the bat, why do we have so many fields in an object? What are we going to do with this gigantic pile of variables?

Now, because this object has values for different currencies, we need to ensure we only work on a single currency. They do this by dynamically constructing the field name with a variable, $the_currency. The code $item -> {"some" . "field"} is a property accessor for, well, "somefield".

On one hand, I hate the dynamic field access to begin with, as obviously this all should be organized differently. On the other, I'm frustrated that they didn't go the next logical step and loop across the two numeric fields. This whole mess would still be a mess, but it'd be a short mess.

All these currency values, and nobody thought to buy an array or two.

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

Sean was supporting a web application which, as many do, had required form fields for the user to fill out. The team wanted to ensure that the required fields were marked by an "*", as you do. Now, there are a lot of ways to potentially accomplish the goal, especially given that the forms are static and the fields are known well ahead of time.

The obvious answer is just including the asterisk directly in the HTML: <label for="myInput">My Input(*)</label>: <input…>. But what if the field requirements change! You'll need to update every field label, potentially. So someone hit upon the "brillant" idea of tracking the names of the fields and their validation requirements in the database. That way, they could output that information when they rendered the page.

Now, again, an obvious solution might be to output it directly into the rendered HTML. But someone decided that they should, instead, use a CSS class to mark it. Not a bad call, honestly! You could style your input.required fields, and even use the ::before or ::after pseudoelements to inject your "*". And if that's what they'd done, we wouldn't be talking about this. But that's not what they did.

<head> <script type="text/javascript"> $(document).ready(function () { //Adds asterisk on required fields $(".requiredField").prepend("* "); }); </script> </head> <body> <div id="first" class="displayBlock"> <div class="fieldlabel"> <span class="requiredField"></span>First Name:</div> @Html.TextBoxFor(i => Model.Applicant.FirstName) <div class="displayBlock">@Html.ValidationMessageFor(i => Model.Applicant.FirstName)</div> </div> </body>

This is a Razor-based .NET View. You can see, in this trimmed down snippet, that they're not actually using the database fields for remembering which UI elements are required, and instead did just hard-code it into the HTML. And they're not using CSS to style anything; they're using JQuery to select all the .required elements and inject the "*" into them.

This, by the way, is the only reason this application ever uses JQuery. The entire JQuery library dependency was added just to handle required fields. Fields, which we know are required because it's hard-coded into the page body. Which raises the question: why not just hard-code the asterisk too? Or are we too worried about wanting to stop using "*" someday in lieu of "!"?

At this point, the code is fairly old, and no one is willing to okay a change which impacts multiple pages and doesn't involve any newly developed features. So this odd little plug of JQuery for JQuery's sake just sorta sits there, staring at Sean every day. No one wants it there, but no one is going to be the one to remove it.

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

Today's anonymous submitter asks a question: "How do you imagine the rest of the codebase to be like?"

Well, let's look at this snippet of TypeScript and think about it:

.catch((): void => { this.loadFromServerAndShowMessage(); }); } else { this.loadFromServerAndShowMessage(); } }) .catch((): void => { this.loadFromServerAndShowMessage(); }); } else { this.loadFromServer(); } }

From the .catchs, I can tell that we're looking at a series of nested promises, each loading content based on the results of the previous. The requests may fail in two ways- either through an exception (hence the catch calls), or because it doesn't return useful data in some fashion (the else clauses).

Regardless of how it fails, we will loadFromServerAndShowMessage, which implies that we're not expecting to handle failures based on connectivity. That seems like a mistake- if our promise fails for some reason, it's likely because we don't have connectivity, so loadFromServer doesn't seem like a viable operation. Maybe we should look at the exception?

Nah, we don't need to do that.

How do I imagine the rest of the code base? Well, I imagine it as a messy thicket of dependencies and no real modularity, fragile and confusing, and probably riddled with bugs, many of which are difficult or impossible to replicate in a controlled environment, meaning they'll likely never get fixed.

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