It's Labor Day in the US, where we celebrate the labor movement and people who, y'know, do actual work. So let's flip back to an old story, which does a lot of extra work. Original -- Remy

Ben Murphy found a developer who liked to cover all of his bases ... then cover the dug-out ... then the bench. If you think this method to convert input (from 33 to 0.33) is a bit superflous, you should see data validation.

Static Function ConvertPercent(v_value As Double)
  If v_value > 1 Then
    ConvertPercent = v_value / 100
  ElseIf v_value = 1 Then
    ConvertPercent = v_value / 100
  ElseIf v_value < 1 Then
    ConvertPercent = v_value / 100
  ElseIf v_value = -1 Then
    ConvertPercent = v_value / 100
  Else 
    ConvertPercent = v_value
  End If 
End Function

The original article- from 2004!- featured Alex asking for a logo. Instead, let me remind you to submit your WTF. Our stories come from our readers. If nothing else, it's a great chance to anonymously vent about work.

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

https://thedailywtf.com/articles/classic-wtf-covering-all-cases-and-then-some

"Beyond the realm of Null, past the Black Stump, lies the mythical FILE_NOT_FOUND," writes Chris A.

"I know, Zwift, I should have started paying you 50 years ago," Jeff wrote, "But hey, thanks for still giving leeches like me a free ride!"

Drake C. writes, "Must...not...click...forbidden...newsletter!"

"I'm having a hard time picking between these 'Exclusives'. It's a shame they're both scheduled for the same time," wrote Rutger.

"Wait, so is this beer zero raised to hex FF? At that price I'll take 0x02!" wrote Tony B.

Kevin F. writes, "Some weed killers say they're powerful. This one backs up that claim!"

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

https://thedailywtf.com/articles/we-must-explore-the-null

If you want millions in VC funding, mumble the words “machine learning” and “disruption” and they’ll blunder out of the woods to just throw money at your startup.

At its core, ML is really about brute-forcing a statistical model. And today’s code from Norine could have possibly been avoided by applying a little more brute force to the programmer responsible.

This particular ML environment, like many, uses Python to wrap around lower-level objects. The ease of Python coupled with the speed of native/GPU-accelerated code. It has a collection of Model datatypes, and at runtime, it needs to decide which concrete Model type it should instantiate. If you come from an OO background in pretty much any other language, you’re thinking about factory patterns and abstract classes, but that’s not terribly Pythonic. Not that this developer’s solution is Pythonic either.

def choose_model(data, env):
  ModelBase = getattr(import_module(env.modelpath), env.modelname)
  
  class Model(ModelBase):
    def __init__(self, data, env):
      if env.data_save is None:
        if env.counter == 0:
          self.data = data
        else:
          raise ValueError("data unavailable with counter > 0")
      
      else:
        with open(env.data_save, "r") as df:
          self.data = json.load(df)
      ModelBase.__init__(self, **self.data)
  
  return Model(data, env)

This is an example of metaprogramming. We use import_module to dynamically load a module at runtime- potentially smart, because modules may take some time to load, so we shouldn’t load a module we don’t know that we’re going to use. Then, with get_attr, we extract the definition of a class with whatever name is stored in env.modelname.

This is the model class we want to instantiate. But instead of actually instantiating it, we instead create a new derived class, and slap a bunch of logic and file loading into it.

Then we instantiate and return an instance of this dynamically defined derived class.

There are so many things that make me cringe. First, I hate putting file access in the constructor. That’s maybe more personal preference, but I hate constructors which can possibly throw exceptions. See also the raise ValueError, where we explicitly throw exceptions. That’s just me being picky, though, and it’s not like this constructor will ever get called from anywhere else.

More concretely bad, these kinds of dynamically defined classes can have some… unusual effects in Python. For example, in Python2 (which this is), each call to choose_model will tag the returned instance with the same type, regardless of which base class it used. Since this method might potentially be using a different base class depending on the env passed in, that’s asking for confusion. You can route around these problems, but they’re not doing that here.

But far, far more annoying is that the super-class constructor, ModelBase.__init__, isn’t called until the end.

You’ll note that our child class manipulates self.data, and while it’s not pictured here, our base model classes? They also use a property called data, but for a different purpose. So our child class inits a child class property, specifically to build a dictionary of key/value pairs, which it then passes as kwargs, or keyword arguments (the ** operator) to the base class constructor… which then overwrites the self.data our child class was using.

So why do any of that?

Norine changed the code to this simpler, more reliable version, which doesn’t need any metaprogramming or dynamically defined classes:

def choose_model(data, env):
  Model = getattr(import_module(env.modelpath), env.modelname)
  
  if env.data_save is not None:
    with open(env.data_save, "r") as df:
      data = json.load(df)
  elif env.counter != 0:
    raise ValueError('if env.counter > 0 then must use data_save parameter')

  return Model(**data)

Norine adds:

I’m thinking of holding on to the original, and showing it to interviewees like a Rorschach test. What do you see in this? The fragility of a plugin system? The perils of metaprogramming? The hollowness of an overwritten argument? Do you see someone with more cleverness than sense? Or someone intelligent but bored? Or perhaps you see, in the way the superclass init is called, TRWTF: a Python 2 library written within the last 3 years.

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

https://thedailywtf.com/articles/learning-the-hard-way

Trung worked for a Microsoft and .NET framework shop that used AutoMapper to simplify object mapping between tiers. Their application's mapping configuration was performed at startup, as in the following C# snippet:

public void Configure(ConfigurationContext context)
{
AutoMapper.Mapper.CreateMap().AfterMap(Map); 
...
}

where the AfterMap() method's Map delegate was to map discrepancies that AutoMapper couldn't.

One day, a senior dev named Van approached Trung for help. He was repeatedly getting AutoMapper's "Missing type map configuration or unsupported mapping. Mapping types Y -> X ..." error.

Trung frowned a little, wondering what was mysterious about this problem. "You're ... probably missing mapping configuration for Y to X," he said.

"No, I'm not!" Van pointed to his monitor, at the same code snippet above.

Trung shook his head. "That mapping is one-way, from X to Y only. You can create the reverse mapping by using the Bidirectional() extension method. Here ..." He leaned over to type in the addition:

AutoMapper.Mapper.CreateMap()
.AfterMap(Map)
.Bidirectional();

This resolved Van's error. Both men returned to their usual business.

A few weeks later, Van approached Trung again, this time needing help with refactoring due to a base library change. While they huddled over Van's computer and dug through compilation errors, Trung kept seeing strange code within multiple AfterMap() delegates:

void Map(X src, Y desc)
{
desc.QueueId = src.Queue.Id;
src.Queue = Queue.GetById(desc.QueueId);
...
}

"Wait a minute!" Trung reached for the mouse to highlight two such lines and asked, "Why is this here?"

"The mapping is supposed to be bidirectional! Remember?" Van replied. "I’m copying from X to Y, then from Y to X."

Trung resisted the urge to clap a hand to his forehead or mutter something about CS101 and variable-swapping—not that this "swap" was necessary. "You realize you'd have nothing but X after doing that?"

The quizzical look on the senior developer's face assured Trung that Van hadn't realized any such thing.

Trung could only sigh and help Van trudge through the delegates he'd "fixed," working out a better mapping procedure for each.

[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/bidirectional

Networks are complex beasts, and as they grow, they get more complicated. Diagnosing and understanding problems on networks rapidly gets hard. “Fortunately” for the world, IniTech ships one of those tools.

Leonore works on IniTech’s protocol analyzer. As you might imagine, a protocol analyzer gathers a lot of data. In the case of IniTech’s product, the lowest level of data acquisition is frequently sampled voltage measurements over time. And it’s a lot of samples- depending on the protocol in question, it might need samples on the order of nanoseconds.

In Leonore’s case, those raw voltage samples are the “primary data”. Now, there are all sorts of cool things that you can do with that primary data, but those computations become expensive. If your goal is to be able to provide realtime updates to the UI, you can’t do most of those computations- you do those outside of the UI update loop.

But you can do some of them. Things like level crossings and timing information can be built quickly enough for the UI. These values are “secondary data”.

As data is collected, there are a number of other sections of the application which need to be notified: the UI and the various high-level analysis components. Architecturally, Leonore’s team made an event-driven approach to doing this. As data is collected, a DataUpdatedEvent fires. The DataUpdatedEvent fires twice: once for the “primary data” and once for the “secondary data”. These two events always happen in lockstep, and they happen so closely together that, for all other modules in the application, they can safely be considered simultaneous, and no components in the application ever only care about one- they always want to see both the primary and the secondary data.

So, to review: the data collection module outputs a pair of data updated events, one containing primary data, one containing secondary data, and can never do anything else, and these two events could basically be viewed as the same event by everything else in the application.

Which raises a question about this C++/COM enum, used to tag the different events:

  enum DataUpdatedEventType 
  {
    [helpstring("Unknown data type.")] UnknownDataType = 0, 
    [helpstring("Primary data.")] PrimaryData = 1,
    [helpstring("Secondary data.")] SecondaryData = 2,
  };

As stated, the distinction between primary/secondary events is unnecessary. In fact, sending two events makes all the consuming code more complicated, because in many cases, they can’t start working until they’ve received the secondary data, and thus have to cache the primary data until the next event arrives.

But that’s minor. The UnknownDataType is never used. It can never be used. There is no case in which the data collection module will ever output that. There’s no reason why it would ever need to output that. None of the consumers are prepared to handle that- sending an UnknownDataType would almost certainly cause a crash in most configurations.

So why is it there? I’ll let Leonore explain:

The only answer I can give is this: When this was written, half of us didn’t know what we were doing most of the time, and most of us didn’t know what we were doing half of the time. So now there’s an enum in the code base that has never been used and, I would submit, CAN never be used. Or maybe I ought to say SHOULD never be used. I would just delete it, but I’ve never quite been able to bring myself to do so.

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

https://thedailywtf.com/articles/unknown-purpose

Zak S worked for a retailer which, as so often happens, got swallowed up by Initech's retail division. Zak's employer had a big, ugly ERP systems. Initech had a bigger, uglier ERP and once the acquisition happened, they all needed to play nicely together.

These kinds of marriages are always problematic, but this particular one was made more challenging: Zak's company ran their entire ERP system from a cluster of Solaris servers- running on SPARC CPUs. Since upgrading that ERP system to run in any other environment was too expensive to seriously consider, the existing services were kept on life-support (with hardware replacements scrounged from the Vintage Computing section of eBay), while Zak's team was tasked with rebuilding everything- point-of-sale, reporting, finance, inventory and supply chain- atop Initech's ERP system.

The project was launched with the code name "Cold Stone", with Glenn as new CTO. At the project launch, Glenn stressed that, "This is a high impact project, with high visibility throughout the organization, so it's on us to ensure that the deliverables are completed on time, on budget, to provide maximum value to the business and to that end, I'll be starting a series of meetings to plan the meetings and checkpoints we'll use to ensure that we have an action-plan that streamlines our…"

"Cold Stone" launched with a well defined project scope, but about 15 seconds after launch, that scope exploded. New "business critical" systems were discovered under every rock, and every department in the company had a moment of, "Why weren't we consulted on this plan? Our vital business process isn't included in your plan!" Or, "You shouldn't have included us in this plan, because our team isn't interested in a software upgrade, we're going to continue using the existing system until the end of time, thank you very much."

The expanding scope required expanding resources. Anyone with any programming experience more complex than "wrote a cool formula in Excel" was press-ganged into the project. You know how to script sending marketing emails? Get on board. You wrote a shell script to purge old user accounts? Great, you're going to write a plugin to track inventory at retail stores.

The project burned through half a dozen business analysts and three project managers, and that's before the COVID-19 outbreak forced the company to quickly downsize, and squish together several project management roles into one person.

"Fortunately" for Initech, that one person was Edyth, who was one of those employees who has given their entire life over to the company, and refuses to sotp working until the work is done. She was the sort of manager who would schedule meetings at 12:30PM, because she knew no one else would be scheduling meetings during the lunch hour. Or, schedule a half hour meeting at 4:30PM, when the workday ends at 5PM, then let it run long, "Since we're all here anyway, let's keep going." She especially liked to abuse video conferencing for this.

As the big ball of mud grew, the project slowly, slowly eased its way towards completion. And as that deadline approached, Edyth started holding meetings which focused on testing. Which is where Edyth started to raise some concerns.

"Lucy," Edyth said, "I noticed that you've marked the test for integration between the e-commerce site and the IniRewards™ site as not-applicable?"

"Well, yeah," Lucy said. "It says to test IniRewards™ signups on the e-commerce site, but our site doesn't do that. Signups entirely happen on the IniRewards™ site. There isn't really any integration."

"Oh," Edyth said. "So that sounds like it's a Zak thing?"

Zak stared at his screen for a moment. He was responsible for the IniRewards™ site, a port of their pre-acquisition customer rewards system to work with Initech's rewards system. He hadn't written it, but somewhere along the way, he became the owner of it, for reasons which remain murky. "Uh… it's a static link."

Edyth nodded, as if she understood what that meant. "So how long will that take to test? A day? Do you need any special setup for this test?"

"It's… a link. I'll click it."

"Great, yes," Edyth said. "Why don't you write up the test plan document for this user story, and then we'll schedule the test for… next week? Can you do it any earlier?"

"I can do it right now," Zak said.

"No, no," Edyth said. "We need to schedule these tests in advance so you're not interacting with anyone else using the test environment. I'll set up a followup meeting to review your test plan."

Test plans, of course, had a template which needed to be filled out. It was a long document, loaded with boiler plate, for the test to be, "Click the 'Rewards Signup' link in the e-commerce site footer. Expected behavior: the browser navigates to the IniRewards™ home page."

Zak added the document to the project document site, labelled "IniRewards Hyper-Link Test", and waited for the next meeting with Edyth to discuss schedule. This time, Glenn, the CTO was in the meeting.

"This 'Hyper-Link' test sounds very important," Glenn said. He enunciated "hyper-link" like it was a word in a foreign language. "Can we move that up in the schedule? I'd like that done tomorrow."

"I… can do it right now," Zak said. "It won't interact with other tests-"

"No, we shouldn't rush things." Glenn's eyes shifted towards another window as he reviewed the testing schedule. "It looks like there's nothing scheduled for testing between 10AM and 2PM tomorrow. Do you think four hours is enough time? Yes? Great, I'll block that off for you."

Suffice to say, the test passed, and was verified quite thoroughly.

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

https://thedailywtf.com/articles/thoroughly-tested

"My Kindle showed me this for the entire time I read this book. Luckily, page 31 is really exciting!" writes Hans H.

Tim wrote, "Thanks JustPark, I'd love to verify my account! Now...how about that button?"

"I almost managed to uninstall Viber, or did I?" writes Simon T.

Marco wrote, "All I wanted to do was to post a one-time payment on a reputable cloud provider. Now I'm just confused."

Brinio H. wrote, "Somehow I expected my muscles to feel more sore after walking over 382 light-years on one day."

"Here we have PowerBI failing to dispel the perception that 'Business Intelligence' is an oxymoron," writes Craig.

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

https://thedailywtf.com/articles/don-t-leave-this-page

I’m going to open with just one line, just one line from Megan D, before we dig into the story:

public static boolean comparePasswords(char[] password1, char[] password2)

A long time ago, someone wrote a Java 1.4 application. It’s all about getting data out of data files, like CSVs and Excel and XML, and getting it into a database, where it can then be turned into plots and reports. Currently, it has two customers, but boy, there’s a lot of technology invested in it, so the pointy-hairs decided that it needed to be updated so they could sell it to new customers.

The developers played a game of “Not It!” and Megan lost. It wasn’t hard to see why no one wanted to touch this code. The UI section was implemented in code generated by an Eclipse plugin that no longer exists. There was UI code which wasn’t implemented that way, but there were no code paths that actually showed it. The project didn’t have one “do everything” class of utilities- it had many of them.

The real magic was in Database.java. All the data got converted into strings before going into the database, and data got pulled back out as lists of strings- one string per row, prepended with the number of columns in that row. The string would get split up and converted back into the actual real datatypes.

Getting back to our sample line above, Megan adds:

No restrictions on any data in the database, or even input cleaning - little Bobby Tables would have a field day. There are so many issues that the fact that passwords are plaintext barely even registers as a problem.

A common convention used in the database layer is “loop and compare”. Want to check if a username exists in the database? SELECT username FROM users WHERE username = 'someuser', loop across the results, and if the username in the result set matches 'someuser', set a flag to true (set it to false otherwise). Return the flag. And if you're wondering why they need to look at each row instead of just seeing a non-zero number of matches, so am I.

Usernames are not unique, but the username/group combination should be.

Similarly, if you’re logging in, it uses a “loop and compare”. Find all the rows for users with that username. Then, find all the groups for that username. Loop across all the groups and check if any of them match the user trying to log in. Then loop across all the stored- plaintext stored passwords and see if they match.

But that raises the question: how do you tell if two strings match? Just use an equality comparison? Or a .equals? Of course not.

We use “loop and compare” on sequences of rows, so we should also use “loop and compare” on sequences of characters. What could be wrong with that?

/**
   * Compares two given char arrays for equality.
   * 
   * @param password1
   *          The first password to compare.
   * @param password2
   *          The second password to compare.
   * @return True if the passwords are equal false otherwise.
   */
  public static boolean comparePasswords(char[] password1, char[] password2)
  {
    // assume false until prove otherwise
    boolean aSameFlag = false;
    if (password1 != null && password2 != null)
    {
      if (password1.length == password2.length)
      {
        for (int aIndex = 0; aIndex < password1.length; aIndex++)
        {
          aSameFlag = password1[aIndex] == password2[aIndex];
        }
      }
    }
    return aSameFlag;
  }

If the passwords are both non-null, if they’re both the same length, compare them one character at a time. For each character, set the aSameFlag to true if they match, false if they don’t.

Return the aSameFlag.

The end result of this is that only the last letter matters, so from the perspective of this code, there’s no difference between the word “ship” and a more accurate way to describe this code.

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

https://thedailywtf.com/articles/win-by-being-last

If you run a business of any size, you need some sort of resource-management/planning software. Really small businesses use Excel. Medium businesses use Excel. Enterprises use Excel. But in addition to that, the large businesses also pay through the nose for a gigantic ERP system, like Oracle or SAP, that they can wire up to Excel.

Small and medium businesses can’t afford an ERP, but they might want to purchase a management package in the niche realm of “SMB software”- small and medium business software. Much like their larger cousins, these SMB tools have… a different idea of code quality.

Cassandra’s company had deployed such a product, and with it came a slew of tickets. The performance was bad. There were bugs everywhere. While the company provided support, Cassandra’s IT team was expected to also do some diagnosing.

While digging around in one nasty performance problem, Cassandra found that one button in the application would generate and execute this block of SQL code using a SQLCommand object in C#.

DECLARE @tmp TABLE (Id uniqueidentifier)

--{ Dynamic single insert statements, may be in the hundreds. }

IF NOT EXISTS (SELECT TOP 1 1 FROM SomeTable AS st INNER JOIN @tmp t ON t.Id = st.PK)
BEGIN
    INSERT INTO SomeTable (PK, SomeDate) SELECT Id, getdate() as SomeDate FROM @tmp 
END
ELSE 
BEGIN
    UPDATE st
        SET SomeDate = getdate()
        FROM @tmp t
        LEFT JOIN SomeTable AS st ON t.Id = st.PK AND SomeDate = NULL
END

At its core, the purpose of this is to take a temp-table full of rows and perform an “upsert” for all of them: insert if a record with that key doesn’t exist, update if a record with that key does. Now, this code is clearly SQL Server code, so a MERGE handles that.

But okay, maybe they’re trying to be as database agnostic as possible, and don’t want to use something that, while widely supported, has some dialect differences across databases. Fine, but there’s another problem here.

Whoever built this understood that in SQL Server land, cursors are frowned upon, so they didn’t want to iterate across every row. But here’s their problem: some of the records may exist, some of them may not, so they need to check that.

As you saw, this was their approach:

IF NOT EXISTS (SELECT TOP 1 1 FROM SomeTable AS st INNER JOIN @tmp t ON t.Id = st.PK)

This is wrong. This will be true only if none of the rows in the dynamically generated INSERT statements exist in the base table. If some of the rows exist and some don’t, you aren’t going to get the results you were expecting, because this code only goes down one branch: it either inserts or updates.

There are other things wrong with this code. For example, SomeDate = NULL is going to have different behavior based on whether the ANSI_NULLS database flag is OFF (in which case it works), or ON (in which case it doesn’t). There’s a whole lot of caveats about whether you set it at the database level, on the connection string, during your session, but in Cassandra’s example, ANSI_NULLS was ON at the time this ran, so that also didn’t work.

There are other weird choices and performance problems with this code, but the important thing is that this code doesn’t work. This is in a shipped product, installed by over 4,000 businesses (the vendor is quite happy to cite that number in their marketing materials). And it ships with code that can’t work.

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

https://thedailywtf.com/articles/where-to-insert-this

Hanna's co-worker implemented a new service, got it deployed, and then left for vacation someplace where there's no phones or Internet. So, of course, Hanna gets a call from one of the operations folks: "That new service your team deployed keeps crashing on startup, but there's nothing in the log."

Hanna took it on herself to check into the VB.Net code.

Public Class Service

    Private mContinue As Boolean = True
    Private mServiceException As System.Exception = Nothing
    Private mAppSettings As AppSettings

    '// ... snip ... //

    Private Sub DoWork()
        Try
            Dim aboutNowOld As String = ""
            Dim starttime As String = DateTime.Now.AddSeconds(5).ToString("HH:mm")
            While mContinue
                Threading.Thread.Sleep(1000)
                Dim aboutnow As String = DateTime.Now.ToString("HH:mm")
                If starttime = aboutnow And aboutnow <> aboutNowOld Then
                    '// ... snip ... //
                    starttime = DateTime.Now.AddMinutes(mAppSettings.pollingInterval).ToString("HH:mm")
                End If
                aboutNowOld = aboutnow
            End While
        Catch ex As Exception
            mServiceException = ex
        End Try
        If mServiceException IsNot Nothing Then
            EventLog.WriteEntry(mServiceException.ToString, Diagnostics.EventLogEntryType.Error)
            Throw mServiceException
        End If
    End Sub

End Class

Presumably whatever causes the crash is behind one of those "snip"s, but Hanna didn't include that information. Instead, let's focus on our unique way to idle.

First, we pick our starttime to be the minute 5 seconds into the future. Then we enter our work loop. Sleep for one second, and then check which minute we're on. If that minute is our starttime and this loop hasn't run during this minute before, we can get into our actual work (snipped), and then calculate the nextstarttime, based on our app settings.

If there are any exceptions, we break the loop, log and re-throw it- but don't do that from the exception handler. No, we store the exception in a member variable and then if it IsNot Nothing we log it out.

Hanna writes: "After seeing this I gave up immediately before I caused a time paradox. Guess we'll have to wait till she's back from the future to fix it."

It's not quite a paradox, but it's certainly far more complex than it ever needs to be. First, we have the stringly-typed date handling. That's just awful. Then, we have the once-per-second polling, but we except pollingInterval to be in minutes. But AddMinutes takes doubles, so it could be seconds, expressed as fractional minutes. But wait, if we know how long we want to wait between executions, couldn't we just Sleep that long? Why poll every second? Does this job absolutely have to run in the first second of every minute? Even if it does, we could easily calculate that sleep time with reasonable accuracy if we actually looked at the seconds portion of the current time.

The developer who wrote this saw the problem of "execute this code once every polling interval" and just called it a day.

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

https://thedailywtf.com/articles/wait-a-minute

There are a few WTFs in today's story. Let's get the first one out of the way: Jan S downloaded a shell script and ran it as root, without reading it. Now, let's be fair, that's honestly a pretty mild WTF; we've all done something similar, and popular software tools still tell you to install them with a curl … | sh, and then sudo themselves extra permissions in the script.

The software being installed in this case is a tool for accessing Bitlocker encrypted drives from Linux. And the real WTF for this one is the install script, which we'll dig into in a moment. This is not, however, some small scale open source project thrown together by hobbyists, but instead released by Initech's "Data Recovery" business. In this case, this is the open source core of a larger data recovery product- if you're willing to muck around with low level commands and configs, you can do it for free, but if you want a vaguely usable UI, get ready to pony up $40.

With that in mind, let's take a look at the script. We're going to do this in chunks, because nearly everything is wrong. You might think I'm exaggerating, but here's the first two lines of the script:

#!/bin/bash
home_dir="/home/"${USER}"/initech.bitlocker"

That is not how you find out the user's home directory. We'll usually use ${HOME}, or since the shebang tells us this is definitely bash, we could just use ~. Jan also points out that while a username probably shouldn't have a space, it's possible, and since the ${USER} isn't in quotes, this breaks in that case.

echo ${home_dir}
install_dir=$1
if [ ! -d "${install_dir}" ]; then
install_dir=${home_dir}
if [ ! -d "${install_dir}" ]; then
echo "create dir : "${install_dir}
mkdir ${install_dir}

Who wants indentation in their scripts? And if a script supports arguments, should we tell the user about it? Of course not! Just check to see if they supplied an argument, and if they did, we'll treat that as the install directory.

As a bonus, the mkdir line protects people like Jan who run this script as root, at least if their home directory is /root, which is common. When it tries to mkdir /home/root/initech.bitlocker, the script fails there.

echo "Install software to ${install_dir}"
cp -rf ./* ${install_dir}"/"

Once again, the person who wrote this script doesn't seem to understand what the double quotes in Bash are for, but the real magic is the next segment:

echo "Copy runtime environment ..."
sudo cp -f ./libcrypto.so.1.0.0 /usr/lib/
sudo cp -f ./libssl.so.1.0.0 /usr/lib64
sudo cp -f ./libcrypto.so.1.0.0 /usr/lib/
sudo cp -f ./libssl.so.1.0.0 /usr/lib64

Did you have libssl already installed in your system? Well now you have this version! Hope that's okay for you. We like our version of libssl and libcrypto so much we're copying them into your library directories twice. They probably meant to copy libcrypto and libssl to both lib and lib64, but messed up.

Well, that is assuming you already have a lib64 directory, because if you don't, you now have a lib64 file which contains the data from libssl.so.1.0.0.

This is the installer for a piece of software which has been released as part of a product that Initech wants to sell, and they don't successfully install it.

sudo ln -s ${install_dir}/mount.bitlocker /usr/bin/mount.bitlocker
sudo ln -s ${install_dir}/bitlocker.creator /usr/bin/create.bitlocker
sudo ln -s ${install_dir}/activate.sh /usr/bin/initech.bitlocker.active
sudo ln -s ${install_dir}/initech.mount.sh /usr/bin/initech.bitlocker.mount
sudo ln -s ${install_dir}/initech.bitlocker.sh /usr/bin/initech.bitlocker

Hey, here's an install step with no critical mistakes, assuming that no other package or tool has tried to claim those names in /usr/bin, which is probably true (Jan actually checked this using dpkg -S … to see if any packages wanted to use that path).

source /etc/os-release
case $ID in
debian|ubuntu|devuan)
	echo "Installing dependent package - curl ..."
    sudo apt-get install curl -y
	echo "Installing dependent package - openssl ..."
	sudo apt-get install openssl -y
	echo "Installing dependent package - fuse ..."
	sudo apt-get install fuse -y
	echo "Installing dependent package - gksu ..."
	sudo apt-get install gksu -y
    ;;

Here's the first branch of our case. They've learned to indent. They've chosen to slap the -y flag on all the apt-get commands, which means the user isn't going to get a choice about installing these packages, which is mildly annoying. It's also worth noting that sourceing /etc/os-release can be considered harmful, but clearly "not doing harm" isn't high on this script's agenda.

centos|fedora|rhel)
    yumdnf="yum"
    if test "$(echo "$VERSION_ID >= 22" | bc)" -ne 0; then
	yumdnf="dnf"
    fi
	echo "Installing dependent package - curl ..."
    sudo $yumdnf install -y curl
	echo "Installing dependent package - openssl ..."
	sudo $yumdnf install -y openssl
	echo "Installing dependent package - fuse ..."
	sudo $yumdnf install -y fuse3-libs.x86_64
    ;;

So, maybe they just don't think if supports additional indentation? They indent the case fine. I'm not sure what their thinking is.

Speaking of if, look closely at that version check: test "$(echo "$VERSION_ID >= 22" | bc)" -ne 0.

Now, this is almost clever. If your Linux version number uses decimal values, like 18.04, you can't do a simple if [ "$VERSION_ID" -ge 22]…: you'd get an integer expression expected error. So using bc does make sense…ish. It would be good to check if, y'know, bc were actually installed- it probably is, but you don't know- and it might be better to actually think about the purpose of the check.

They don't actually care what version of Redhat Linux you're running. What they're checking is if your version uses yum for package management, or its successor dnf. A more reliable check would be to simply see if dnf is a valid command, and if not, fallback to yum.

Let's finish out the case statement:

*)
    exit 1
    ;;
esac

So if your system doesn't use an apt based package manager or a yum/dnf based package manager, this just bails at this point. No error message, just an error number. You know it failed, and you don't know why, and it failed after copying a bunch of crap around your system.

So first it mostly installs itself, then it checks to see if it can successfully install all of its dependencies. And if it fails, does it clean up the changes it made? You better believe it doesn't!

echo ""
echo "Initech BitLocker Loader has been installed to "${install_dir}" successfully."
echo "Run initech.bitlocker --help to learn more about Initech BitLocker Loader"

This is a pretty optimistic statement, and while yes, it has theoretically been installed to ${install_dir}, assuming that we've gotten this far, it's really installed to your /usr/bin directory.

The real extra super-special puzzle to me is that it interfaces with your package manager to install dependencies. But it also installs its own versions of libcrypto and libssl, which don't come from your package manager. Ignoring the fact that it probably *installs them into the wrong places*, it seems bad. Suspicious, bad, and troubling.

Jan didn't send us the uninstall script, and honestly, I assume there isn't one. But if there is one, you know it probably tries to do rm -rf /${SOME_VAR_THAT_MIGHT_BE_EMPTY} somewhere in there. Which, in consideration, is probably the safest way to uninstall this software anyway.

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

https://thedailywtf.com/articles/sudon-t

"Sure thing Google, I guess I'll change my language to... let's see...Ah, how about English?" writes Peter G.

Marcus K. wrote, "Breaking news: tt tttt tt,ttt!"

Tim Y. writes, "Nothing makes my day more than someone accidentially leaving testing mode enabled (and yes, the test number went through!)"

"I guess even thinning brows and psoriasis can turn political these days," Lawrence W. wrote.

Strahd I. writes, "It was evident at the time that King Georges VI should have gone asked for a V12 instead."

"Well, gee, ZDNet, why do you think I enabled this setting in the first place?" Jeroen V. writes.

[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/just-a-suggestion

Aurelia is working on a project where some of the code comes from a client. In this case, it appears that the client has very good reasons for hiring an outside vendor to actually build the application.

Imagine you have some Java code which needs to take an array of integers and iterate across them in reverse, to concatenate a string. Oh, and you need to add one to each item as you do this.

You might be thinking about some combination of a map/reverseString.join operation, or maybe a for loop with a i-- type decrementer.

I’m almost certain you aren’t thinking about this.

public String getResultString(int numResults) {
	StringBuffer sb = null;
	
	for (int result[] = getResults(numResults); numResults-- > 0;) {
		int i = result[numResults];
		if( i == 0){
			int j = i + 1; 
			if (sb == null)
				sb = new StringBuffer();
			else
				sb.append(",");
				sb.append(j);
		}else{
			int j = i + 1; 
			if (sb == null)
				sb = new StringBuffer();
			else
				sb.append(",");
				sb.append(j);
		}
	}
	return sb.toString();
}

I really, really want you to look at that for loop: for (int result[] = getResults(numResults); numResults-- > 0;)

Just look at that. It’s… not wrong. It’s not… bad. It’s just written by an alien wearing a human skin suit. Our initializer actually populates the array we’re going to iterate across. Our bounds check also contains the decrement operation. We don’t have a decrement clause.

Then, if i == 0 we’ll do the exact same thing as if i isn’t 0, since our if and else branches contain the same code.

Increment i, and store the result in j. Why we don’t use the ++i or some other variation to be in-line with our weird for loop, I don’t know. Maybe they were done showing off.

Then, if our StringBuffer is null, we create one, otherwise we append a ",". This is one solution to the contatenator’s comma problem. Again, it’s not wrong, it’s just… unusual.

But this brings us to the thing which is actually, objectively, honestly bad. The indenting.

			if (sb == null)
				sb = new StringBuffer();
			else
				sb.append(",");
				sb.append(j);

Look at that last line. Does that make you angry? Look more closely. Look for the curly brackets. Oh, you don’t see any? Very briefly, when I was looking at this code, I thought, “Wait, does this discard the first item?” No, it just eschews brackets and then indents wrong to make sure we’re nice and confused when we look at the code.

It should read:

			if (sb == null)
				sb = new StringBuffer();
			else
				sb.append(",");
                        sb.append(j);

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

https://thedailywtf.com/articles/a-backwards-for

There are times where someone writes code which does nothing. There are times where someone writes code which does something, but nothing useful. This is one of those times.

Ray H was going through some JS code, and found this “useful” method.

mapRowData (data) {
  if (isNullOrUndefined(data)) return null;
  return data.map(x => x);
}

Technically, this isn’t a “do nothing” method. It converts undefined values to null, and it returns a shallow copy of an array, assuming that you passed in an array.

The fact that it can return a null value or an array is one of those little nuisances that we accept, but probably should code around (without more context, it’s probably fine if this returned an empty array on bad inputs, for example).

But Ray adds: “Where this is used, it could just use the array data directly and get the same result.” Yes, it’s used in a handful of places, and in each of those places, there’s no functional difference between the original array and the shallow copy.

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

https://thedailywtf.com/articles/a-shallow-perspective

I avoid writing software that needs to send emails. It's just annoying code to build, interfacing with mailservers is shockingly frustrating, and honestly, users don't tend to like the emails that my software tends to send. Once upon a time, it was a system which would tell them it was time to calibrate a scale, and the business requirements were basically "spam them like three times a day the week a scale comes do," which shockingly everyone hated.

But Krista inherited some code that sends email. The previous developer was a "senior", but probably could have had a little more supervision and maybe some mentoring on the C# language.

One commit added this method, for sending emails:

    private void SendEmail(ExportData exportData, String subject, String fileName1, String fileName2)
    {
        try
        {
            if (String.IsNullOrEmpty(exportData.Email))
            {
                WriteToLog("No email address - message not sent");
            }
            else
            {

                MailMessage mailMsg = new MailMessage();

                mailMsg.To.Add(new MailAddress(exportData.Email, exportData.PersonName));
                mailMsg.Subject = subject;
                mailMsg.Body = "Exported files attached";
                mailMsg.Priority = MailPriority.High;
                mailMsg.BodyEncoding = Encoding.ASCII;
                mailMsg.IsBodyHtml = true;

                if (!String.IsNullOrEmpty(exportData.EmailCC))
                {
                    string[] ccAddress = exportData.EmailCC.Split(';');
                    foreach (string address in ccAddress)
                    {
                        mailMsg.CC.Add(new MailAddress(address));
                    }
                }

                if (File.Exists(fileName1)) mailMsg.Attachments.Add(new Attachment(fileName1));
                if (File.Exists(fileName2)) mailMsg.Attachments.Add(new Attachment(fileName2));

                send(mailMsg);

                mailMsg.Dispose();
            }
        }
        catch (Exception ex)
        {
            WriteToLog(ex.ToString());
        }
    }

That's not so bad, as these things go, though one has to wonder about parameters like fileName1 and fileName2. Do they only ever send exactly two files? Well, maybe when this method was written, but a few commits later, an overloaded version gets added:

    private void SendEmail(ExportData exportData, String subject, String fileName1, String fileName2, String fileName3)
    {
        try
        {
            if (String.IsNullOrEmpty(exportData.Email))
            {
                WriteToLog("No email address - message not sent");
            }
            else
            {

                MailMessage mailMsg = new MailMessage();

                mailMsg.To.Add(new MailAddress(exportData.Email, exportData.PersonName));
                mailMsg.Subject = subject;
                mailMsg.Body = "Exported files attached";
                mailMsg.Priority = MailPriority.High;
                mailMsg.BodyEncoding = Encoding.ASCII;
                mailMsg.IsBodyHtml = true;

                if (!String.IsNullOrEmpty(exportData.EmailCC))
                {
                    string[] ccAddress = exportData.EmailCC.Split(';');
                    foreach (string address in ccAddress)
                    {
                        mailMsg.CC.Add(new MailAddress(address));
                    }
                }

                if (File.Exists(fileName1)) mailMsg.Attachments.Add(new Attachment(fileName1));
                if (File.Exists(fileName2)) mailMsg.Attachments.Add(new Attachment(fileName2));
                if (File.Exists(fileName3)) mailMsg.Attachments.Add(new Attachment(fileName3));

                send(mailMsg);

                mailMsg.Dispose();
            }
        }
        catch (Exception ex)
        {
            WriteToLog(ex.ToString());
        }
    }

And then, a few commits later, someone decided that they needed to send four files, sometimes.

    private void SendEmail(ExportData exportData, String subject, String fileName1, String fileName2, String fileName3, String fileName4)
    {
        try
        {
            if (String.IsNullOrEmpty(exportData.Email))
            {
                WriteToLog("No email address - message not sent");
            }
            else
            {

                MailMessage mailMsg = new MailMessage();

                mailMsg.To.Add(new MailAddress(exportData.Email, exportData.PersonName));
                mailMsg.Subject = subject;
                mailMsg.Body = "Exported files attached";
                mailMsg.Priority = MailPriority.High;
                mailMsg.BodyEncoding = Encoding.ASCII;
                mailMsg.IsBodyHtml = true;

                if (!String.IsNullOrEmpty(exportData.EmailCC))
                {
                    string[] ccAddress = exportData.EmailCC.Split(';');
                    foreach (string address in ccAddress)
                    {
                        mailMsg.CC.Add(new MailAddress(address));
                    }
                }

                if (File.Exists(fileName1)) mailMsg.Attachments.Add(new Attachment(fileName1));
                if (File.Exists(fileName2)) mailMsg.Attachments.Add(new Attachment(fileName2));
                if (File.Exists(fileName3)) mailMsg.Attachments.Add(new Attachment(fileName3));
                if (File.Exists(fileName4)) mailMsg.Attachments.Add(new Attachment(fileName4));

                send(mailMsg);

                mailMsg.Dispose();
            }
        }
        catch (Exception ex)
        {
            WriteToLog(ex.ToString());
        }
    }

Each time someone discovered a new case where they wanted to include a different number of attachments, the previous developer copy/pasted the same code, with minor revisions.

Krista wrote a single version which used a paramarray, which replaced all of these versions (and any other possible versions), without changing the calling semantics.

Though the real WTF is probably still forcing the BodyEncoding to be ASCII at this point in time. There's a whole lot of assumptions about your dataset which are probably not true, or at least no reliably true.

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

https://thedailywtf.com/articles/carbon-copy

Tomiko* inherited some web-scraping/indexing code from Dennis. The code started out just scanning candidate profiles for certain keywords, but grew, mutated, and eventually turned into something that also needed to download their CVs.

Now, Dennis was, as Tomiko puts it, "an interesting engineer". "Any agreed upon standard, he would aggressively oppose, and this can be seen in this code."

"This code" also happens to be in Perl, the "best" language for developers who don't like standards. And, it also happens to be connected to this infrastructure.

So let's start with the code, because this is the rare CodeSOD where the code itself isn't the WTF:

    foreach my $n (0 .. @{$lines} - 1)
    {
        next if index($lines->[$n], 'RT::Spider::Deepweb::Controller::Factory->make(') == -1;

        # Don't let other cv_id survive.
        $lines->[$n] =~ s/,\s*cv_id\s*=>[^,)]+//;
        $lines->[$n] =~ s/,\s*cv_type\s*=>[^,)]+// if defined $cv_type;

        # Insert the new options.
        $lines->[$n] =~ s/\)/$opt)/;
    }

Okay, so it's a pretty standard for-each loop. We skip lines if they contain… wait, that looks like a Perl expression- RT::Spider::Deepweb::Controller::Factory->make('? Well, let's hold onto that thought, but keep trucking on.

Next, we do a few find-and-replace operations to ensure that we Don't let other cv_id survive. I'm not really sure what exactly that's supposed to mean, but Tomiko says, "Dennis never wrote a single meaningful comment".

Well, the regexes are pretty standard character-salad expressions; ugly, but harmless. If you take this code in isolation, it's not good, but it doesn't look terrible. Except, there's that next if line. Why are we checking to see if the input data contains a Perl expression?

Because our input data is a Perl script. Dennis was… efficient. He already had code that would download the candidate profiles. Instead of adding new code to download CVs, instead of refactoring the existing code so that it was generic enough to download both, Dennis instead decided to load the profile code into memory, scan it with regexes, and then eval it.

As Tomiko says: "You can't get more Perl than that."

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

https://thedailywtf.com/articles/perls-can-change

"Honest! If I could give you something that had a 'cat' in it, I would!" wrote Gordon P.

"You'd think Outlook would hage told me sooner about these required updates," Carlos writes.

Colin writes, "Asking for a friend, does balsamic olive oil still have to be changed every 3,000 miles?"

"I was looking for Raspberry Pi 4 cases on my local Amazon.co.jp when I stumbled upon a pretty standard, boring WTF. Desparate to find an actual picture of the case I was after, I changed to Amazon.com and I guess I got what I wanted," George wrote. (Here are the short versions: https://www.amazon.co.jp/dp/B07TFDFGZFhttps://www.amazon.com/dp/B07TFDFGZF)

Kevin wrote, "Ah, I get it. Shiny and blinky ads are SO last decade. Real container advertisers nowadays get straight to the point!"

"I noticed this in the footer of an email from my apartment management company and well, I'm intrigued at the possibility of 'rewards'," wrote Peter C.

[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/new-cat-nullness

Don’t you believe in magic strings and numbers being bad? From the perspective of readability and future maintenance, constants are better. We all know this is true, and we all know that it can sometimes go too far.

Douwe Kasemier has a co-worker that has taken that a little too far.

For example, they have a Java method with a signature like this:

Document addDocument(Action act, boolean createNotification);

The Action type contains information about what action to actually perform, but it will result in a Document. Sometimes this creates a notification, and sometimes it doesn’t.

Douwe’s co-worker was worried about the readability of addDocument(myAct, true) and addDocument(myAct, false), so they went ahead and added some constants:

    private static final boolean NO_NOTIFICATION = false;
    private static final boolean CREATE_NOTIFICATION = true;

Okay, now, I don’t love this, but it’s not the worst thing…

public Document doActionWithNotification(Action act) {
  addDocument(act, CREATE_NOTIFICATION);
}

public Document doActionWithoutNotification(Action act) {
  addDocument(act, NO_NOTIFICATION);
}

Okay, now we’re just getting silly. This is at least diminishing returns of readability, if not actively harmful to making the code clear.

    private static final int SIX = 6;
    private static final int FIVE = 5;
    public String findId(String path) {
      String[] folders = path.split("/");
      if (folders.length >= SIX && (folders[FIVE].startsWith(PREFIX_SR) || folders[FIVE].startsWith(PREFIX_BR))) {
          return folders[FIVE].substring(PREFIX_SR.length());
      }
      return null;
    }

Ah, there we go. The logical conclusion: constants for 5 and 6. And yet they didn’t feel the need to make a constant for "/"?

At least this in maintainable, so that when the value of FIVE changes, the method doesn’t need to change.

[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/don-t-stop-the-magic

In the spring of 2020, with very little warning, every school in the United States shut down due to the ongoing global pandemic. Classrooms had to move to virtual meeting software like Zoom, which was never intended to be used as the primary means of educating grade schoolers. The teachers did wonderfully with such little notice, and most kids finished out the year with at least a little more knowledge than they started. This story takes place years before then, when online schooling was seen as an optional add-on and not a necessary backup plan in case of plague.

TelEdu provided their take on such a thing in the form of a free third-party add-on for Moodle, a popular e-learning platform. Moodle provides space for teachers to upload recordings and handouts; TelEdu takes it one step further by adding a "virtual classroom" complete with a virtual whiteboard. The catch? You have to pay a subscription fee to use the free module, otherwise it's nonfunctional.

Initech decided they were on a tight schedule to implement a virtual classroom feature for their corporate training, so they went ahead and bought the service without testing it. They then scheduled a demonstration to the client, still without testing it. The client's 10-man team all joined to test out the functionality, and it wasn't long before the phone started ringing off the hook with complaints: slowness, 504 errors, blank pages, the whole nine yards.

That's where Paul comes in to our story. Paul was tasked with finding what had gone wrong and completing the integration. The most common complaint was that Moodle was being slow, but upon testing it himself, Paul found that only the TelEdu module pages were slow, not the rest of the install. So far so good. The code was open-source, so he went digging through to find out what in view.php was taking so long:

$getplan = telEdu_get_plan();
$paymentinfo = telEdu_get_payment_info();
$getclassdetail = telEdu_get_class($telEduclass->class_id);
$pricelist = telEdu_get_price_list($telEduclass->class_id);

Four calls to get info about the class, three of them to do with payment. Not a great start, but not necessarily terrible, either. So, how was the info fetched?

function telEdu_get_plan() {
    $data['task'] = TELEDU_TASK_GET_PLAN;
    $result = telEdu_get_curl_info($data);
    return $result;
}

"They couldn't possibly ... could they?" Paul wondered aloud.

function telEdu_get_payment_info() {
    $data['task'] = TELEDU_TASK_GET_PAYMENT_INFO;
    $result = telEdu_get_curl_info($data);
    return $result;
}

Just to make sure, Paul next checked what telEdu_get_curl_info actually did:

function telEdu_get_curl_info($data) {
    global $CFG;
    require_once($CFG->libdir . '/filelib.php');

    $key = $CFG->mod_telEdu_apikey;
    $baseurl = $CFG->mod_telEdu_baseurl;

    $urlfirstpart = $baseurl . "/" . $data['task'] . "?apikey=" . $key;

    if (($data['task'] == TELEDU_TASK_GET_PAYMENT_INFO) || ($data['task'] == TELEDU_TASK_GET_PLAN)) {
        $location = $baseurl;
    } else {
        $location = telEdu_post_url($urlfirstpart, $data);
    }

    $postdata = '';
    if ($data['task'] == TELEDU_TASK_GET_PAYMENT_INFO) {
        $postdata = 'task=getPaymentInfo&apikey=' . $key;
    } else if ($data['task'] == TELEDU_TASK_GET_PLAN) {
        $postdata = 'task=getplan&apikey=' . $key;
    }

    $options = array(
        'CURLOPT_RETURNTRANSFER' => true, 'CURLOPT_SSL_VERIFYHOST' => false, 'CURLOPT_SSL_VERIFYPEER' => false,
    );

    $curl = new curl();
    $result = $curl->post($location, $postdata, $options);

    $finalresult = json_decode($result, true);
    return $finalresult;
}

A remote call to another API using, of all things, a shell call out to cURL, which queried URLs from the command line. Then it waited for the result, which was clocking in at anywhere between 1 and 30 seconds ... each call. The result wasn't used anywhere, either. It seemed to be just a precaution in case somewhere down the line they wanted these things.

After another half a day of digging through the rest of the codebase, Paul gave up. Sales told the client that "Due to the high number of users, we need more time to make a small server calibration."

The calibration? Replacing TelEdu with BigBlueButton. Problem solved.

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

https://thedailywtf.com/articles/teleconference-horror

In English, there's much debate over the "Oxford Comma": in a list of items, do you put a comma between the penultimate item and the "and" before the final one? For example: "The conference featured bad programmers, Remy and TheDailyWTF readers" versus "The conference featured bad programmers, Remy, and the TheDailyWTF readers."

I'd like to introduce a subtly different one: "the concatenator's comma", or if we want to be generic "the concatenator's seperator character", but that doesn't have the same ring to it. If you're planning to list items as a string, you might to something like this pseudocode:

  for each item in items
    result.append(item + ", ")

This naive approach does pose a problem: we'll have an extra comma. So maybe you have to add logic to decide if you're on the first or last item, and insert (or fail to insert) commas as appropriate. Or, maybe isn't a problem- if we're generating JSON, for example, we can just leave the trailing commas. This isn't universally true, of course, but many formats will ignore extra separators. Edit: I was apparently hallucinating when I wrote this; one of the most annoying things about JSON is that you can't do this.

Like, for example, URL query strings, which don't require a "sub-delim" like "&" to have anything following it.

But fortunately for us, no matter what language we're using, there's almost certainly an API that makes it so that we don't have to do string concatenation anyway, so why even bring it up?

Well, because Mike has a co-worker that has read the docs well enough to know that PHP has a substr method, but not well enough to know it has an http_build_query method. Or even an implode method, which handles string concats for you. Instead, they wrote this:

$query = '';
foreach ($postdata as $var => $val) {
$query .= $var .'='. $val .'&';
}
$query = substr($query, 0, -1);

This code exploits a little-observed feature of substr: a negative length reads back from the end. So this lops off that trailing "&", which is both unnecessary and one of the most annoying ways to do this.

Maybe it's not enough to RTFM, as Mike puts it, maybe you need to "RTEFM": read the entire manual.

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

https://thedailywtf.com/articles/the-concatenator