Gordon S. is smarter than the machines. "I can only presume the "Fix Now with AI" button adds some mistakes in order to fix the lack of needed fixes."

 

"Sorry, repost with the link https://www.daybreaker.com/alive/," wrote Michael R.

 

And yet again from Michael R., following up with a package mistracker. "Poor DHL driver. I hope he will get a break within those 2 days. And why does the van look like he's driving away from me."

 

Morgan airs some dirty laundry. "After navigating this washing machine app on holiday and validating my credit card against another app I am greeted by this less than helpful message each time. So is OK okay? Or is the Error in error?
Washing machine worked though."

 

And finally, scamproof Stuart wondered "Maybe the filter saw the word "scam" and immediately filed it into the scam bucket. All scams include the word "scam" in them, right?"

 

[Advertisement] Keep all your packages and Docker containers in one place, scan for vulnerabilities, and control who can access different feeds. ProGet installs in minutes and has a powerful free version with a lot of great features that you can upgrade when ready.Learn more.

Optional types are an attempt to patch the "billion dollar mistake". When you don't know if you have a value or not, you wrap it in an Optional, which ensures that there is a value (the Optional itself), thus avoiding null reference exceptions. Then you can query the Optional to see if there is a real value or not.

This is all fine and good, and can cut down on some bugs. Good implementations are loaded with convenience methods which make it easy to work on the optionals.

But then, you get code like Burgers found. Which just leaves us scratching our heads:

private static final Optional<Boolean> TRUE = Optional.of(Boolean.TRUE); private static final Optional<Boolean> FALSE = Optional.of(Boolean.FALSE);

Look, any time you're making constants for TRUE or FALSE, something has gone wrong, and yes, I'm including pre-1999 versions of C in this. It's especially telling when you do it in a language that already has such constants, though- at its core- these lines are saying TRUE = TRUE. Yes, we're wrapping the whole thing in an Optional here, which potentially is useful, but if it is useful, something else has gone wrong.

Burgers works for a large insurance company, and writes this about the code:

I was trying to track down a certain piece of code in a Spring web API application when I noticed something curious. It looked like there was a chunk of code implementing an application-specific request filter in business logic, totally ignoring the filter functions offered by the framework itself and while it was not related to the task I was working on, I followed the filter apply call to its declaration. While I cannot supply the entire custom request filter implementation, take these two static declarations as a demonstration of how awful the rest of the class is.

Ah, of course- deep down, someone saw a perfectly functional wheel and said, "I could make one of those myself!" and these lines are representative of the result.

[Advertisement] Keep all your packages and Docker containers in one place, scan for vulnerabilities, and control who can access different feeds. ProGet installs in minutes and has a powerful free version with a lot of great features that you can upgrade when ready.Learn more.

Matt was handed a pile of VB .Net code, and told, "This is yours now. I'm sorry."

As often happens, previous company leadership said, "Why should I pay top dollar for experienced software engineers when I can hire three kids out of college for the same price?" The experiment ended poorly, and the result was a pile of bad VB code, which Matt now owned.

Here's a little taste:

// SET IN SESSION AND REDIRECT TO PRINT PAGE Session["PrintValue"] = GenerateHTMLOfItem(); Response.Redirect("PrintItem.aspx", true);

The function name here is accurate. GenerateHTMLOfItem takes an item ID, generates the HTML output we want to use to render the item, and stores it in a session variable. It then forces the browser to redirect to a different page, where that HTML can then be output.

You may note, of course, that GenerateHTMLOfItem doesn't actually take parameters. That's because the item ID got stored in the session variable elsewhere.

Of course, it's the redirect that gets all the attention here. This is a client side redirect, so we generate all the HTML, shove it into a session object, and then send a message to the web browser: "Go look over here". The browser sends a fresh HTTP request for the new page, at which point we render it for them.

The Microsoft documentation also has this to add about the use of Response.Redirect(String, Boolean), as well:

Calling Redirect(String) is equivalent to calling Redirect(String, Boolean) with the second parameter set to true. Redirect calls End which throws a ThreadAbortException exception upon completion. This exception has a detrimental effect on Web application performance. Therefore, we recommend that instead of this overload you use the HttpResponse.Redirect(String, Boolean) overload and pass false for the endResponse parameter, and then call the CompleteRequest method. For more information, see the End method.

I love it when I see the developers do a bonus wrong.

Matt had enough fires to put out that fixing this particular disaster wasn't highest on his priority list. For the time being, he could only add this comment:

// SET IN SESSION AND REDIRECT TO PRINT PAGE // FOR THE LOVE OF GOD, WHY?!? Session["PrintValue"] = GenerateHTMLOfItem(); Response.Redirect("PrintItem.aspx", true); [Advertisement] Keep all your packages and Docker containers in one place, scan for vulnerabilities, and control who can access different feeds. ProGet installs in minutes and has a powerful free version with a lot of great features that you can upgrade when ready.Learn more.

Today's awfulness comes from Tim H, and while it's technically more than one line, it's so representative of the code, and so short that I'm going to call this a representative line. Before we get to the code, we need to talk a little history.

Tim's project is roughly three decades old. It's a C++ tool used for a variety of research projects, and this means that 90% of the people who have worked on it are PhD candidates in computer science programs. We all know the rule of CompSci PhDs and programming: they're terrible at it. It's like the old joke about the farmer who, when unable to find an engineer to build him a cow conveyer, asked a physicist. After months of work, the physicist introduced the result: "First, we assume a perfectly spherical cow in a vacuum…"

Now, this particularly function has been anonymized, but it's easy to understand what the intent was:

bool isFooOrBar() { return isFoo() && isBar(); }

The obvious problem here is the mismatch between the function name and the actual function behavior- it promises an or operation, but does an and, which the astute reader may note are different things.

I think this offers another problem, though. Even if the function name were correct, given the brevity of the body, I'd argue that it actually makes the code less clear. Maybe it's just me, but isFoo() && isBar() is more clear in its intent than isFooAndBar(). There's a cognitive overhead to adding more symbols that would make me reluctant to add such a function.

There may be an argument about code-reuse, but it's worth noting- this function is only ever called in one place.

This particular function is not itself, all that new. Tim writes:

This was committed as new code in 2010 (i.e., not a refactor). I'm not sure if the author changed their mind in the middle of writing the function or just forgot which buttons on the keyboard to press.

More likely, Tim, is that they initially wrote it as an "or" operation and then discovered that they were wrong and it needed to be an "and". Despite the fact that the function was only called in one place, they opted to change the body without changing the name, because they didn't want to "track down all the places it's used". Besides, isn't the point of a function to encapsulate the behavior?

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

Everyone's got workplace woes. The clueless manager; the disruptive coworker; the cube walls that loom ever higher as the years pass, trapping whatever's left of your soul.

But sometimes, Satan really leaves his mark on a joint. I worked Tech Support there. This is my story. Who am I? Just call me Anonymous.

It starts at the top. A call came in from Lawrence Gibbs, the CEO himself, telling us that a conference room printer was, quote, "leaking." He didn't explain it, he just hung up. The boss ordered me out immediately, told me to step on it. I ignored the elevator, racing up the staircase floor after floor until I reached the dizzying summit of C-Town.

There's less oxygen up there, I'm sure of it. My lungs ached and my head spun as I struggled to catch my breath. The fancy tile and high ceilings made a workaday schmuck like me feel daunted, unwelcome. All the same, I gathered myself and pushed on, if only to learn what on earth "leaking" meant in relation to a printer.

I followed the signs on the wall to the specified conference room. In there, the thermostat had been kicked down into the negatives. The cold cut through every layer of mandated business attire, straight to bone. The scene was thick with milling bystanders who hugged themselves and traded the occasional nervous glance. Gibbs was nowhere to be found.

Remembering my duty, I summoned my nerve. "Tech Support. Where's the printer?" I asked.

Several pointing fingers showed me the way. The large printer/scanner was situated against the far wall, flanking an even more enormous conference table. Upon rounding the table, I was greeted with a grim sight: dozens of sheets of paper strewn about the floor like blood spatter. Everyone was keeping their distance; no one paid me any mind as I knelt to gather the pages. There were 30 in all. Each one was blank on one side, and sported some kind of large, blotchy ring on the other. Lord knew I drank enough java to recognize a coffee mug stain when I saw one, but these weren't actual stains. They were printouts of stains.

The printer was plugged in. No sign of foul play. As I knelt there, unseen and unheeded, I clutched the ruined papers to my chest. Someone had wasted a tree and a good bit of toner, and for what? How'd it go down? Surely Gibbs knew more than he'd let on. The thought of seeking him out, demanding answers, set my heart to pounding. It was no good, I knew. He'd play coy all day and hand me my pink slip if I pushed too hard. As much as I wanted the truth, I had a stack of unpaid bills at home almost as thick as the one in my arms. I had to come up with something else.

There had to be witnesses among the bystanders. I stood up and glanced among them, seeking out any who would return eye contact. There: a woman who looked every bit as polished as everyone else. But for once, I got the feeling that what lay beneath the facade wasn't rotten.

With my eyes, I pleaded for answers.

Not here, her gaze pleaded back.

I was getting somewhere, I just had to arrange for some privacy. I hurried around the table again and weaved through bystanders toward the exit, hoping to beat it out of that icebox unnoticed. When I reached the threshold, I spotted Gibbs charging up the corridor, smoldering with entitlement. "Where the hell is Tech Support?!"

I froze a good distance away from the oncoming executive, whose voice I recognized from a thousand corporate presentations. Instead of putting me to sleep this time, it jolted down my spine like lightning. I had to think fast, or I was gonna lose my lead, if not my life.

"I'm right here, sir!" I said. "Be right back! I, uh, just need to find a folder for these papers."

"I've got one in my office."

A woman's voice issued calmly only a few feet behind me. I spun around, and it was her, all right, her demeanor as cool as our surroundings. She nodded my way. "Follow me."

My spirits soared. At that moment, I would've followed her into hell. Turning around, I had the pleasure of seeing Gibbs stop short with a glare of contempt. Then he waved us out of his sight.

Once we were out in the corridor, she took the lead, guiding me through the halls as I marveled at my luck. Eventually, she used her key card on one of the massive oak doors, and in we went.

You could've fit my entire apartment into that office. The place was spotless. Mini-fridge, espresso machine, even couches: none of it looked used. There were a couple of cardboard boxes piled up near her desk, which sat in front of a massive floor-to-ceiling window admitting ample sunlight.

She motioned toward one of the couches, inviting me to sit. I shook my head in reply. I was dying for a cigarette by that point, but I didn't dare light up within this sanctuary. Not sure what to expect next, I played it cautious, hovering close to the exit. "Thanks for the help back there, ma'am."

"Don't mention it." She walked back to her desk, opened up a drawer, and pulled out a brand-new manila folder. Then she returned to conversational distance and proffered it my way. "You're from Tech Support?"

There was pure curiosity in her voice, no disparagement, which was encouraging. I accepted the folder and stuffed the ruined pages inside. "That's right, ma'am."

She shook her head. "Please call me Leila. I started a few weeks ago. I'm the new head of HR."

That acronym, which usually put me on edge, somehow failed to raise my hackles. I'd have to keep vigilant, of course, but so far she seemed surprisingly OK. "Welcome aboard, Leila. I wish we were meeting in better circumstances." Duty beckoned. I hefted the folder. "Printers don't just leak."

"No." Leila glanced askance, grave.

"Tell me what you saw."

"Well ..." She shrugged helplessly. "Whenever Mr. Gibbs gets excited during a meeting, he tends to lean against the printer and rest his coffee mug on top of it. Today, he must've hit the Scan button with his elbow. I saw the scanner go off. It was so bright ..." She trailed off with a pained glance downward.

"I know this is hard," I told her when the silence stretched too long. "Please, continue."

Leila summoned her mettle. "After he leaned on the controls, those pages spilled out of the printer. And then ... then somehow, I have no idea, I swear! Somehow, all those pages were also emailed to me, Mr. Gibbs' assistant, and the entire board of directors!"

The shock hit me first. My eyes went wide and my jaw fell. But then I reminded myself, I'd seen just as crazy and worse as the result of a cat jumping on a keyboard. A feline doesn't know any better. A top-level executive, on the other hand, should know better.

"Sounds to me like the printer's just fine," I spoke with conviction. "What we have here is a CEO who thinks it's OK to treat an expensive piece of office equipment like his own personal fainting couch."

"It's terrible!" Leila's gaze burned with purpose. "I promise, I'll do everything I possibly can to make sure something like this never happens again!"

I smiled a gallows smile. "Not sure what anyone can do to fix this joint, but the offer's appreciated. Thanks again for your help."

Now that I'd seen this glimpse of better things, I selfishly wanted to linger. But it was high time I got outta there. I didn't wanna make her late for some meeting or waste her time. I backed up toward the door on feet that were reluctant to move.

Leila watched me with a look of concern. "Mr. Gibbs was the one who called Tech Support. I can't close your ticket for you; you'll have to get him to do it. What are you going to do?"

She cared. That made leaving even harder. "I dunno yet. I'll think of something."

I turned around, opened the massive door, and put myself on the other side of it in a hurry, using wall signs to backtrack to the conference room. Would our paths ever cross again? Unlikely. Someone like her was sure to get fired, or quit out of frustration, or get corrupted over time.

It was too painful to think about, so I forced myself to focus on the folder of wasted pages in my arms instead. It felt like a mile-long rap sheet. I was dealing with an alleged leader who went so far as to blame the material world around him rather than accept personal responsibility. I'd have to appeal to one or more of the things he actually cared about: himself, his bottom line, his sense of power.

By the time I returned to the conference room to face the CEO, I knew what to tell him. "You're right, sir, there's something very wrong with this printer. We're gonna take it out here and give it a thorough work-up."

That was how I was able to get the printer out of that conference room for good. Once it underwent "inspection" and "testing," it received a new home in a previously unused closet. Whenever Gibbs got to jawing in future meetings, all he could do was lean against the wall. Ticket closed.

Gibbs remained at the top, doing accursed things that trickled down to the roots of his accursed company. But at least from then on, every onboarding slideshow included a photo of one of the coffee ring printouts, with the title Respect the Equipment.

Thanks, Leila. I can live with that.

[Advertisement] Picking up NuGet is easy. Getting good at it takes time. Download our guide to learn the best practice of NuGet for the Enterprise.

"What word can spell with the letters housucops?" asks Mark R. "Sometimes AI hallucinations can be hard to find. Other times, they just kind of stand out..."

 

"Do I need more disks?" wonders Gordon "I'm replacing a machine which has only 2 GB of HDD. New one has 2 TB, but that may not be enough. Unless Thunar is lying." It's being replaced by an LLM.

 

"Greenmobility UX is a nightmare" complains an anonymous reader. "Just like last week's submission, do you want to cancel? Cancel or Leave?" This is not quite as bad as last week's.

 

Cinephile jeffphi rated this film two thumbs down. "This was a very boring preview, cannot recommend."

 

Malingering Manuel H. muses "Who doesn't like long weekends? Sometimes, one Sunday per week is just not enough, so just put a second one right after the first." I don't want to wait until Oktober for a second Sunday; hope we get one søøn.

 

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

Once upon a time, when the Web was young, if you wanted to be a cool kid, you absolutely needed two things on your website: a guestbook for people to sign, and a hit counter showing how many people had visited your Geocities page hosting your Star Trek fan fiction.

These days, we don't see them as often, but companies still like to track the information, especially when it comes to counting downloads. So when Justin started on a new team and saw a download count in their analytics, he didn't think much of it at all. Nor did he think much about it when he saw the download count displayed on the download page.

Another thing that Justin didn't think much about was big piles of commits getting merged in overnight, at least not at first. But each morning, Justin needed to pull in a long litany of changes from a user named "MrStinky". For the first few weeks, Justin was too preoccupied with getting his feet under him, so he didn't think about it too much.

But eventually, he couldn't ignore what he saw in the git logs.

docs: update download count to 51741 docs: update download count to 51740 docs: update download count to 51738

And each commit was exactly what the name implied, a diff like:

- 51740 + 51741

Each time a user clicked the download link, a ping was sent to their analytics system. Throughout the day, the bot "MrStinky" would query the analytics tool, and create new commits that updated the counter. Overnight, it would bundle those commits into a merge request, approve the request, merge the changes, and then redeploy what was at the tip of main.

"But, WHY?" Justin asked his peers.

One of them just shrugged. "It seemed like the easiest and fastest way at the time?"

"I wanted to wire Mr Stinky up to our content management system's database, but just never got around to it. And this works fine," said another.

Much like the rest of the team, Justin found that there were bigger issues to tackle.

[Advertisement] Plan Your .NET 9 Migration with Confidence
Your journey to .NET 9 is more than just one decision.Avoid migration migraines with the advice in this free guide. Download Free Guide Now!

Jessica recently started at a company still using Windows Forms.

Well, that was a short article. Oh, you want more WTF than that? Sure, we can do that.

As you might imagine, a company that's still using Windows Forms isn't going to upgrade any time soon; they've been using an API that's been in maintenance mode for a decade, clearly they're happy with it.

But they're not too happy- Jessica was asked to track down a badly performing report. This of course meant wading through a thicket of spaghetti code, pointless singletons, and the general sloppiness that is the code base. Some of the code was written using Entity Framework for database access, much of it is not.

While it wasn't the report that Jessica was sent to debug, this method caught her eye:

private Dictionary<long, decimal> GetReportDiscounts(ReportCriteria criteria) { Dictionary<long, decimal> rows = new Dictionary<long, decimal>(); string query = @"select ii.IID, SUM(CASE WHEN ii.AdjustedTotal IS NULL THEN (ii.UnitPrice * ii.Units) ELSE ii.AdjustedTotal END) as 'Costs' from ii where ItemType = 3 group by ii.IID "; string connectionString = string.Empty; using (DataContext db = DataContextFactory.GetInstance<DataContext>()) { connectionString = db.Database.Connection.ConnectionString; } using (SqlConnection connection = new SqlConnection(connectionString)) { using (SqlCommand command = new SqlCommand(query, connection)) { command.Parameters.AddWithValue("@DateStart", criteria.Period.Value.Min.Value.Date); command.Parameters.AddWithValue("@DateEnd", criteria.Period.Value.Max.Value.Date.AddDays(1)); command.Connection.Open(); using (SqlDataReader reader = command.ExecuteReader()) { while (reader.Read()) { decimal discount = (decimal)reader["Costs"]; long IID = (long)reader["IID"]; if (rows.ContainsKey(IID)) { rows[IID] += discount; } else { rows.Add(IID, discount); } } } } } return rows; }

This code constructs a query, opens a connection, runs the query, and iterates across the results, building a dictionary as its result set. The first thing which leaps out is that, in code, they're doing a summary (iterating across the results and grouping by IID), which is also what they did in the query.

It's also notable that the table they're querying is called ii, which is not a result of anonymization, and actually what they called it. Then there's the fact that they set parameters on the query, for DateStart and DateEnd, but the query doesn't use those. And then there's that magic number 3 in the query, which is its own set of questions.

Then, right beneath that method was one called GetReportTotals. I won't share it, because it's identical to what's above, with one difference:

string query = @" select ii.IID, SUM(CASE WHEN ii.AdjustedTotal IS NULL THEN (ii.UnitPrice * ii.Units) ELSE ii.AdjustedTotal END) as 'Costs' from ii where itemtype = 0 group by iid ";

The magic number is now zero.

So, clearly we're in the world of copy/paste programming, but this raises the question: which came first, the 0 or the 3? The answer is neither. GetCancelledInvoices came first.

private List<ReportDataRow> GetCancelledInvoices(ReportCriteria criteria, Dictionary<long, string> dictOfInfo) { List<ReportDataRow> rows = new List<ReportDataRow>(); string fCriteriaName = "All"; string query = @"select A long query that could easily be done in EF, or at worst a stored procedure or view. Does actually use the associated parameters"; string connectionString = string.Empty; using (DataContext db = DataContextFactory.GetInstance<DataContext>()) { connectionString = db.Database.Connection.ConnectionString; } using (SqlConnection connection = new SqlConnection(connectionString)) { using (SqlCommand command = new SqlCommand(query, connection)) { command.Parameters.AddWithValue("@DateStart", criteria.Period.Value.Min.Value.Date); command.Parameters.AddWithValue("@DateEnd", criteria.Period.Value.Max.Value.Date.AddDays(1)); command.Connection.Open(); using (SqlDataReader reader = command.ExecuteReader()) { while (reader.Read()) { long ID = (long)reader["ID"]; decimal costs = (decimal)reader["Costs"]; string mNumber = (string)reader["MNumber"]; string mName = (string)reader["MName"]; DateTime idate = (DateTime)reader["IDate"]; DateTime lastUpdatedOn = (DateTime)reader["LastUpdatedOn"]; string iNumber = reader["INumber"] is DBNull ? string.Empty : (string)reader["INumber"]; long fId = (long)reader["FID"]; string empName = (string)reader["EmpName"]; string empNumber = reader["EmpNumber"] is DBNull ? string.Empty : (string)reader["empNumber"]; long mId = (long)reader["MID"]; string cName = dictOfInfo[matterId]; if (criteria.EmployeeID.HasValue && fId != criteria.EmployeeID.Value) { continue; } rows.Add(new ReportDataRow() { CName = cName, IID = ID, Costs = costs * -1, //Cancelled i - minus PC TimedValue = 0, MNumber = mNumber, MName = mName, BillDate = lastUpdatedOn, BillNumber = iNumber + "A", FID = fId, EmployeeName = empName, EmployeeNumber = empNumber }); } } } } return rows; }

This is the original version of the method. We can infer this because it actually uses the parameters of DateStart and DateEnd. Everything else just copy/pasted this method and stripped out bits until it worked. There are more children of this method, each an ugly baby of its own, but all alike in their ugliness.

It's also worth noting, the original version is doing filtering after getting data from the database, instead of putting that criteria in the WHERE clause.

As for Jessica's poor performing report, it wasn't one of these methods. It was, however, another variation on "run a query, then filter, sort, and summarize in C#". By simply rewriting it as a SQL query in a stored procedure that leveraged indexes, performance improved significantly.

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

In theory, HTTP status codes should be easy to work with. In the 100s? You're doing some weird stuff and breaking up large requests into multiple sub-requests. 200s? It's all good. 300s? Look over there. 400s? What the hell are you trying to do? 500s? What the hell is the server trying to do?

This doesn't mean people don't endlessly find ways to make it hard. LinkedIn, for example, apparently likes to send 999s if you try and view a page without being logged in. Shopify has invented a few. Apache has added a 218 "This is Fine". And then there's WebDAV, which not only adds new status codes, but adds a whole bunch of new verbs to HTTP requests.

Francesco D sends us a "clever" attempt at handling status codes.

try { HttpRequest.Builder localVarRequestBuilder = {{operationId}}RequestBuilder({{#allParams}}{{paramName}}{{^-last}}, {{/-last}}{{/allParams}}{{#hasParams}}, {{/hasParams}}headers); return memberVarHttpClient.sendAsync( localVarRequestBuilder.build(), HttpResponse.BodyHandlers.ofString()).thenComposeAsync(localVarResponse -> { if (localVarResponse.statusCode()/ 100 != 2) { return CompletableFuture.failedFuture(getApiException("{{operationId}}", localVarResponse)); } {{#returnType}} try { String responseBody = localVarResponse.body(); return CompletableFuture.completedFuture( responseBody == null || responseBody.isBlank() ? null : memberVarObjectMapper.readValue(responseBody, new TypeReference<{{{returnType}}}>() {}) ); } catch (IOException e) { return CompletableFuture.failedFuture(new ApiException(e)); } {{/returnType}} {{^returnType}} return CompletableFuture.completedFuture(null); {{/returnType}} }); }

Okay, before we get to the status code nonsense, I first have to whine about this templating language. I'm generally of the mind that generated code is a sign of bad abstractions, especially if we're talking about using a text templating engine, like this. I'm fine with hygienic macros, and even C++'s templating system for code generation, because they exist within the language. But fine, that's just my "ok boomer" opinion, so let's get into the real meat of it, which is this line:

localVarResponse.statusCode()/ 100 != 2

"Hey," some developer said, "since success is in the 200 range, I'll just divide by 100, and check if it's a 2, helpfully truncating the details." Which is fine and good, except neither 100s nor 300s represent a true error, especially because if the local client is doing caching, a 304 tells us that we can used the cached version.

For Francesco, treating 300s as an error created a slew of failed requests which shouldn't have failed. It wasn't too difficult to detect- they were at least logging the entire response- but it was frustrating, if only because it seems like someone was more interested in being clever with math than actually writing good software.

[Advertisement] Keep all your packages and Docker containers in one place, scan for vulnerabilities, and control who can access different feeds. ProGet installs in minutes and has a powerful free version with a lot of great features that you can upgrade when ready.Learn more.

For months, everything at Yusuf's company was fine. Then, suddenly, he comes in to the office to learn that overnight the log exploded with thousands of panic messages. No software changes had been pushed, no major configurations had happened- just a reboot. What had gone wrong?

This particular function was invoked as part of the application startup:

func (a *App) setupDocDBClient(ctx context.Context) error { docdbClient, err := docdb.NewClient( ctx, a.config.MongoConfig.URI, a.config.MongoConfig.Database, a.config.MongoConfig.EnableTLS, ) if err != nil { return nil } a.DocDBClient = docdbClient return nil }

This is Go, which passes errors as part of the return. You can see an example where docdb.NewClient returns a client and an err object. At one point in the history of this function, it did the same thing- if connecting to the database failed, it returned an error.

But a few months earlier, an engineer changed it to swallow the error- if an error occurred, it would return nil.

As an organization, they did code reviews. Multiple people looked at this and signed off- or, more likely, multiple people clicked a button to say they'd looked at it, but hadn't.

Most of the time, there weren't any connection issues. But sometimes there were. One reboot had a flaky moment with connecting, and the error was ignored. Later on in execution, downstream modules started failing, which eventually lead to a log full of panic level messages.

The change was part of a commit tagged merely: "Refactoring". Something got factored, good and hard, all right.

[Advertisement] Picking up NuGet is easy. Getting good at it takes time. Download our guide to learn the best practice of NuGet for the Enterprise.

low-case jeffphi found "Yep, all kinds of technical errors."

 

Michael R. reports an off by 900 error.

 

"It is often said that news slows down in August," notes Stewart , wondering if "perhaps The Times have just given up? Or perhaps one of the biggest media companies just doesn't care about their paying subscribers?"

 

"Zero is a dangerous idea!" exclaims Ernie in Berkeley .

 

Daniel D. found one of my unfavorites, calling it "Another classic case of cancel dialog. This time featuring KDE Partition Manager."

 

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

Andreas found this in a rather large, rather ugly production code base.

private static void LogView(object o) { try { ArrayList al = (ArrayList)o; int pageId = (int)al[0]; int userId = (int)al[1]; // ... snipped: Executing a stored procedure that stores the values in the database } catch (Exception) { } }

This function accepts an object of any type, except no, it doesn't, it expect that object to be an ArrayList. It then assumes the array list will then store values in a specific order. Note that they're not using a generic ArrayList here, nor could they- it (potentially) needs to hold a mix of types.

What they've done here is replace a parameter list with an ArrayList, giving up compile time type checking for surprising runtime exceptions. And why?

"Well," the culprit explained when Andreas asked about this, "the underlying database may change. And then the function would need to take different parameters. But that could break existing code, so this allows us to add parameters without ever having to change existing code."

"Have you heard of optional arguments?" Andreas asked.

"No, all of our arguments are required. We'll just default the ones that the caller doesn't supply."

And yes, this particular pattern shows up all through the code base. It's "more flexible this way."

.comment { border: none; } [Advertisement] Keep all your packages and Docker containers in one place, scan for vulnerabilities, and control who can access different feeds. ProGet installs in minutes and has a powerful free version with a lot of great features that you can upgrade when ready.Learn more.

Ronan works with a vibe coder- an LLM addicted developer. This is a type of developer that's showing up with increasing frequency. Their common features include: not reading the code the AI generated, not testing the code the AI generated, not understanding the context of the code or how it integrates into the broader program, and absolutely not bothering to follow the company coding standards.

Here's an example of the kind of Python code they were "writing":

if isinstance(o, Test): if o.requirement is None: logger.error(f"Invalid 'requirement' in Test: {o.key}") try: raise ValueError("Missing requirement in Test object.") except ValueError: pass if o.title is None: logger.error(f"Invalid 'title' in Test: {o.key}") try: raise ValueError("Missing title in Test object.") except ValueError: pass

An isinstance check, is already a red flag. Even without proper type annotations and type checking (though you should use them) any sort of sane coding is going to avoid situations where your method isn't sure what input it's getting. isinstance isn't a WTF, but it's a hint at something lurking off screen. (Yes, sometimes you do need it, this may be one of those times, but I doubt it.)

In this case, if the Test object is missing certain fields, we want to log errors about it. That part, honestly, is all fine. There are potentially better ways to express this idea, but the idea is fine.

No, the obvious turd in the punchbowl here is the exception handling. This is pure LLM, in that it's a statistically probable result of telling the LLM "raise an error if the requirement field is missing". The resulting code, however, raises an exception, immediately catches it, and then does nothing with it.

I'd almost think it's a pre-canned snippet that's meant to be filled in, but no- there's no reason a snippet would throw and catch the same error.

Now, in Ronan's case, this has a happy ending: after a few weeks of some pretty miserable collaboration, the new developer got fired. None of "their" code ever got merged in. But they've already got a few thousand AI generated resumes out to new positions…

[Advertisement] Keep all your packages and Docker containers in one place, scan for vulnerabilities, and control who can access different feeds. ProGet installs in minutes and has a powerful free version with a lot of great features that you can upgrade when ready.Learn more.

JavaScript is frequently surprising in terms of what functions it does not support. For example, while it has a Math.round function, that only rounds to the nearest integer, not an arbitrary precision. That's no big deal, of course, as if you wanted to round to, say, four decimal places, you could write something like: Math.floor(n * 10000) / 10000.

But in the absence of a built-in function to handle that means that many developers choose to reinvent the wheel. Ryan found this one.

function stripExtraNumbers(num) { //check if the number's already okay //assume a whole number is valid var n2 = num.toString(); if(n2.indexOf(".") == -1) { return num; } //if it has numbers after the decimal point, //limit the number of digits after the decimal point to 4 //we use parseFloat if strings are passed into the method if(typeof num == "string"){ num = parseFloat(num).toFixed(4); } else { num = num.toFixed(4); } //strip any extra zeros return parseFloat(num.toString().replace(/0*$/,"")); }

We start by turning the number into a string and checking for a decimal point. If it doesn't have one, we've already rounded off, return the input. Now, we don't trust our input, so if the input was already a string, we'll parse it into a number. Once we know it's a number, we can call toFixed, which returns a string rounded off to the correct number of decimal points.

This is all very dumb. Just dumb. But it's the last line which gets really dumb.

toFixed returns a padded string, e.g. (10).toFixed(4) returns "10.0000". But this function doesn't want those trailing zeros, so they convert our string num into a string, then use a regex to replace all of the trailing zeros, and then parse it back into a float.

Which, of course, when storing the number as a number, we don't really care about trailing zeros. That's a formatting choice when we output it.

I'm always impressed by a code sample where every single line is wrong. It's like a little treat. In this case, it even gets me a sense of how it evolved from little snippets of misunderstood code. The regex to remove trailing zeros in some other place in this developer's experience led to degenerate cases where they had output like 10., so they also knew they needed to have the check at the top to see if the input had a fractional part. Which the only way they knew to do that was by looking for a . in a string (have fun internationalizing that!). They also clearly don't have a good grasp on types, so it makes sense that they have the extra string check, just to be on the safe side (though it's worth noting that parseFloat is perfectly happy to run on a value that's already a float).

This all could be a one-liner, or maybe two if you really need to verify your types. Yet here we are, with a delightfully wrong way to do everything.

.comment { border: none; } [Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.

We've discussed singleton abuse as an antipattern many times on this site, but folks keep trying to find new ways to implement them badly. And Olivia's co-worker certainly found one.

We start with a C++ utility class with a bunch of functions in it:

//utilities.h class CUtilities { public CUtilities(); void doSomething(); void doSomeOtherThing(); }; extern CUtilities* g_Utility;

So yes, if you're making a pile of utility methods, or if you want a singleton object, the keyword you're looking for is static. We'll set that aside. This class declares a class, and then also declares that there will be a pointer to the class, somewhere.

We don't have to look far.

//utilities.cpp CUtilities* g_Utility = nullptr; CUtilities::CUtilities() { g_Utility = this; } // all my do-whatever functions here

This defines the global pointer variable, and then also writes the constructor of the utility class so that it initializes the global pointer to itself.

It's worth noting, at this point, that this is not a singleton, because this does nothing to prevent multiple instances from being created. What it does guarantee is that for each new instance, we overwrite g_Utility without disposing of what was already in there, which is a nice memory leak.

But where, or where, does the constructor get called?

//startup.h class CUtilityInit { private: CUtilities m_Instance; }; //startup.cpp CUtilityInit *utils = new CUtilityInit();

I don't hate a program that starts with an initialization step that clearly instantiates all the key objects. There's just one little problem here that we'll come back to in just a moment, but let's look at the end result.

Anywhere that needs the utilities now can do this:

#include "utilities.h" //in the code g_Utility->doSomething();

There's just one key problem: back in the startup.h, we have a private member called CUtilities m_Instance which is never referenced anywhere else in the code. This means when people, like Olivia, are trawling through the codebase looking for linter errors they can fix, they may see an "unused member" and decide to remove it. Which is what Olivia did.

The result compiles just fine, but explodes at runtime since g_Utility was never initialized.

The fix was simple: just don't try and make this a singleton, since it isn't one anyway. At startup, she just populated g_Utility with an instance, and threw away all the weird code around populating it through construction.

Singletons are, as a general rule, bad. Badly implemented singletons themselves easily turn into landmines waiting for unwary developers. Stop being clever and don't try and apply a design pattern for the sake of saying you used a design pattern.

.comment { border: none; } [Advertisement] Picking up NuGet is easy. Getting good at it takes time. Download our guide to learn the best practice of NuGet for the Enterprise.

It is said (allegedly by the Scots) that confession heals the soul. But does it need to be strictly voluntary? The folks toiling away over at CodeSOD conscientiously change the names to protect the innocent but this side of the house is committed to curing the tortured souls of webdevs. Whether they like it or not. Sadly Sam's submission has been blinded, so the black black soul of xxxxxxxxxxte.com remains unfortunately undesmirched, but three others should be experiencing the sweet sweet healing light of day right about now. I sure hope they appreciate it.

More monkey business this week from Reinier B. who is hoping to check in on some distant cousins. "I'll make sure to accept email from {email address}, otherwise I won't be able to visit {zoo name}."

 

Alex A. is "trying to pay customs duty." It definitely can be.

 

"I know it's hard to recruit good developers," commiserates Sam B. , "but it's like they're not even trying." They sure are, as above.

 

Peter G. bemoans "Apparently this network power thingamajig, found on Aliexpress, is pain itself if the brand name on it is to be believed." Cicero wept.

 

Jan B. takes the perfecta, hitting not only this week's theme of flubstitutions but also a bounty of bodged null references to boot. "This is one of the hardest choices I've ever had in my life. I'm not sure if I'd prefer null or null as my location.detection.message.CZ." Go in peace.

 

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

"Code should be clear and explain what it does, comments should explain why it does that." This aphorism is a decent enough guideline, though like any guidance short enough to fit on a bumper sticker, it can easily be overapplied or misapplied.

Today, we're going to look at a comment Salagir wrote. This comment does explain what the code does, can't hope to explain why, and instead serves as a cautionary tale. We're going to take the comment in sections, because it's that long.

This is about a stored procedure in MariaDB. Think of Salagir as our Virgil, a guide showing us around the circles of hell. The first circle? A warning that the dead code will remain in the code base:

/************************** Dead code, but don't delete! What follows if the history of a terrible, terrible code. I keep it for future generations. Read it in a cold evening in front of the fireplace.

My default stance is "just delete bad, dead code". But it does mean we get this story out of it, so for now I'll allow it.

**** XXX **** This is the story of the stored procedure for getext_fields. **** XXX **** Gets the english and asked language for the field, returns what it finds: it's the translation you want. Called like this: " SELECT getext('$table.$field', $key, '$lang') as $label " The function is only *in the database you work on right now*.

Okay, this seems like a pretty simple function. But why does this say "the function is only in the database you work on right now"? That's concerning.

***** About syntax!! The code below can NOT be used by copy and paste in SQL admin (like phpmyadmin), due to the multiple-query that needs DELIMITER set. The code that works in phpmyadmin is this: DELIMITER $$ DROP FUNCTION IF EXISTS getext$$ CREATE FUNCTION (...same...) LIMIT 1; RETURN `txt_out`; END$$ However, DELIMITER breaks the code when executed from PHP.

Am I drowning in the river Styx? Why would I be copy/pasting SQL code into PhpMyAdmin from my PHP code? Is… is this a thing people were doing? Or was it going the opposite way, and people were writing delimited statements and hoping to execute them as a single query? I'm not surprised that didn't work.

***** About configuration!!! IMPORTANT: If you have two MySQL servers bind in Replication mode in order to be able to execute this code, you (or your admin) should set: SET GLOBAL log_bin_trust_function_creators = 1; Without that, adding of this function will fail (without any error).

I don't know the depths of MariaDB, so I can't comment on if this is a WTF. What leaps out to me though, is that this likely needs to be in a higher-level form of documentation, since this is a high-level configuration flag. Having it live here is a bit buried. But, this is dead code, so it's fine, I suppose.

***** About indexes!!!! The primary key was not used as index in the first version of this function. No key was used. Because the code you see here is modified for it's execution. And `field`=my_field becomes `field`= NAME_CONST('my_field',_ascii'[value]' COLLATE 'ascii_bin') And if the type of my_field in the function parameter wasn't the exact same as the type of `text`, no index is used! At first, I didn't specify the charset, and it became `field`= NAME_CONST('my_field',_utf8'[value]' COLLATE 'utf8_unicode_ci') Because utf8 is my default, and no index was used, the table `getext_fields` was read entirely each time! Be careful of your types and charsets... Also...

Because the code you see here is modified for its execution. What? NAME_CONST is meant to create synthetic columns not pulled from tables, e.g. SELECT NAME_CONST("foo", "bar") would create a result set with one column ("foo"), with one row ("bar"). I guess this is fine as part of a join- but the idea that the code written in the function gets modified before execution is a skin-peelingly bad idea. And if the query is rewritten before being sent to the database, I bet that makes debugging hard.

***** About trying to debug!!!!! To see what the query becomes, there is *no simple way*. I literally looped on a SHOW PROCESSLIST to see it! Bonus: if you created the function with mysql user "root" and use it with user "SomeName", it works. But if you do the show processlist with "SomeName", you won't see it!!

Ah, yes, of course. I love running queries against the database without knowing what they are, and having to use diagnostic tools in the database to hope to understand what I'm doing.

***** The final straw!!!!!! When we migrated to MariaDB, when calling this a lot, we had sometimes the procedure call stucked, and UNKILLABLE even on reboot. To fix it, we had to ENTIRELY DESTROY THE DATABASE AND CREATE IT BACK FROM THE SLAVE. Several times in the same month!!!

This is the 9th circle of hell, reserved for traitors and people who mix tabs and spaces in the same file. Unkillable even on reboot? How do you even do that? I have a hunch about the database trying to retain consistency even after failures, but what the hell are they doing inside this function creation statement that can break the database that hard? The good news(?) is the comment(!) contains some of the code that was used:

**** XXX **** The creation actual code, was: **** XXX **** // What DB are we in? $PGf = $Phoenix['Getext']['fields']; $db = $PGf['sql_database']? : ( $PGf['sql_connection'][3]? : ( $sql->query2cell("SELECT DATABASE()") ) ); $func = $sql->query2assoc("SHOW FUNCTION STATUS WHERE `name`='getext' AND `db`='".$sql->e($db)."'"); if ( !count($func) ) { $sql->query(<<<MYSQL CREATE FUNCTION {$sql->gt_db}getext(my_field VARCHAR(255) charset {$ascii}, my_id INT(10) UNSIGNED, my_lang VARCHAR(6) charset {$ascii}) RETURNS TEXT DETERMINISTIC BEGIN DECLARE `txt_out` TEXT; SELECT `text` INTO `txt_out` FROM {$sql->gt_db}`getext_fields` WHERE `field`=my_field AND `id`=my_id AND `lang` IN ('en',my_lang) AND `text`!='' ORDER BY IF(`lang`=my_lang, 0, 1) LIMIT 1; RETURN `txt_out`; END; MYSQL ); ... }

I hate doing string munging to generate SQL statements, but I especially hate it when the very name of the object created is dynamic. The actual query doesn't look too unreasonable, but everything about how we got here is terrifying.

**** XXX **** Today, this is not used anymore, because... **** XXX **** Because a simple sub-query perfectly works! And no maria-db bug. Thus, in the function selects() The code: //example: getext('character.name', `character_id`, 'fr') as name $sels[] = $this->sql_fields->gt_db."getext('$table.$field', $key, '$lang') as `$label`"; Is now: $sels[] = "(SELECT `text` FROM {$this->sql_fields->gt_db}`getext_fields` WHERE `field`='$table.$field' AND `lang` IN ('en', '$lang') AND `id`=$key AND `text`!='' ORDER BY IF(`lang`='$lang', 0, 1) LIMIT 1) as `$label`"; Less nice to look at, but no procedure, all the previous problems GONE! **** XXX The end. */

Of course a simple subquery (or heck, probably a join!) could handle this. Linking data across two tables is what databases are extremely good at. I agree that, at the call site, this is less readable, but there are plenty of ways one could clean this up to make it more readable. Heck, with this, it looks a heck of a lot like you could have written a much simpler function.

Salagir did not provide the entirety of the code, just this comment. The comment remains in the code, as a warning sign. That said, it's a bit verbose. I think a simple "Abandon all hope, ye who enter here," would have covered it.

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

While I frequently have complaints about over-reliance on Object Relational Mapping tools, they do offer key benefits. For example, mapping each relation in the database to a type in your programming language at least guarantees a bit of type safety in your code. Or, you could be like Nick L's predecessor, and write VB code like this.

For i As Integer = 0 To SQLDataset.Tables(0).Rows.Count - 1 Try 'Handles DBNull Select Case SQLDataset.Tables(0).Rows(i).Item(0) Case "Bently" 'Probes Probes_Combobox.Items.Add(SQLDataset.Tables(0).Rows(i).Item(1).ToUpper.ToString.Trim) Case "Keyphasor" Keyphasor_Combobox.Items.Add(SQLDataset.Tables(0).Rows(i).Item(1).ToUpper.ToString.Trim) Case "Transmitter" Transmitter_Combobox.Items.Add(SQLDataset.Tables(0).Rows(i).Item(1).ToUpper.ToString.Trim) Case "Tachometer" Tachometer_Combobox.Items.Add(SQLDataset.Tables(0).Rows(i).Item(1).ToUpper.ToString.Trim.ToUpper.ToString.Trim) Case "Dial Therm" DialThermometer_Combobox.Items.Add(SQLDataset.Tables(0).Rows(i).Item(1).ToUpper.ToString.Trim) Case "DPS" DPS_Combobox.Items.Add(SQLDataset.Tables(0).Rows(i).Item(1).ToUpper.ToString.Trim) Case "Pump Bracket" PumpBracket_Combobox.Items.Add(SQLDataset.Tables(0).Rows(i).Item(1).ToUpper.ToString.Trim) Case "Accelerometer" Accelerometer_Combobox.Items.Add(SQLDataset.Tables(0).Rows(i).Item(1).ToUpper.ToString.Trim) Case "Velometer" Velometer_Combobox.Items.Add(SQLDataset.Tables(0).Rows(i).Item(1).ToUpper.ToString.Trim) End Select Catch 'MessageBox.Show(text:="Error during SetModelNums().", _ ' caption:="Error", _ ' buttons:=MessageBoxButtons.OK, _ ' icon:=MessageBoxIcon.Error) End Try Next

So, for starters, they're using the ADO .Net DataSet object. This is specifically meant to be a disconnected, in-memory model of the database. The idea is that you might run a set of queries, store the results in a DataSet, and interact with the data entirely in memory after that point. The resulting DataSet will model all the tables and constraints you've pulled in (or allow you to define your own in memory).

One of the things that the DataSet tracks is the names of tables. So, the fact that they go and access .Table(0) is a nuisance- they could have used the name of the table. And while that might have been awfully verbose, there's nothing stopping them from doing DataTable products = SQLDataSet.Tables("Products").

None of this is what caught Nick's attention, though. You see, the DataTable in the DataSet will do its best to map database fields to .NET types. So it's the chain of calls at the end of most every field that caught Nick's eye:

SQLDataset.Tables(0).Rows(i).Item(1).ToUpper.ToString.Trim

ToUpper works because the field in the database is a string field. Also, it returns a string, so there's no need to ToString it before trimming. Of course, it's the Tachometer entry that brings this to its natural absurdity:

Tachometer_Combobox.Items.Add(SQLDataset.Tables(0).Rows(i).Item(1).ToUpper.ToString.Trim.ToUpper.ToString.Trim)

All of this is wrapped up in an exception handler, not because of the risk of an error connecting to the database (the DataSet is disconnected after all), but because of the risk of null values, as the comment helpfully states.

We can see that once, this exception handler displayed a message box, but that has since been commented out, presumably because there are a lot of nulls and the number of message boxes the users had to click through were cumbersome. Now, the exception handler doesn't actually check what kind of exception we get, and just assumes the only thing that could happen was a null value. But that's not true- someone changed one of the tables to add a column to the front, which meant Item(1) was no longer grabbing the field the code expects, breaking the population of the Pump Bracket combo box. There was no indication that this had happened beyond users asking, "Why are there no pump brackets anymore?"

[Advertisement] Plan Your .NET 9 Migration with Confidence
Your journey to .NET 9 is more than just one decision.Avoid migration migraines with the advice in this free guide. Download Free Guide Now!

Michael has the "fun" task of converting old, mainframe-driven reports into something more modern. This means reading through reams of Intelligent Query code.

Like most of these projects, no one has a precise functional definition of what it's supposed to do. The goal is to replace the system with one that behaves exactly the same, but is more "modern". This means their test cases are "run the two systems in parallel and compare the outputs; if they match, the upgrade is good."

After converting one report, the results did not match. Michael dug in, tracing through the code. The name of the report contained the word "Annual". One of the key variables which drove original the report was named TODAYS-365 (yes, you can put dashes in variables in IQ). Michael verified that the upgraded report was pulling exactly one year's worth of data. Tracing through the original report, Michael found this:

# DIVIDE ISBLCS BY ISB-COST-UOM GIVING ISB-COST-EACH. MULTIPLY ISB-STK-QOH TIMES ISB-COST-EACH GIVING ISB-ON-HAND-COST. # SUBTRACT TODAYS-DATE MINUS 426 GIVING TODAYS-365. # SEARCH FOR ITMMAN = 'USA' AND ITMNMB <> '112-*'

This snippet comes from a report which contains many hundreds of lines of code. So it's very easy to understand how someone could miss the important part of the code. Specifically, it's this line: SUBTRACT TODAYS-DATE MINUS 426 GIVING TODAYS-365..

Subtract 426 from today's date, and store the result in a variable called TODAYS-365. This report isn't for the past year, but for the past year and about two months.

It's impossible to know exactly why, but at a guess, originally the report needed to grab a year. Then, at some point, the requirement changed, probably based on some nonsense around fiscal years or something similar. The least invasive way to make that change was to just change the calculation, leaving the variable name (and the report name) incorrect and misleading. And there it say, working perfectly fine, until poor Michael came along, trying to understand the code.

The fix was easy, but the repeated pattern of oddly name, unclear variables was not. Remember, the hard part about working on old mainframes isn't learning COBOL or IQ or JCL or whatever antique languages they use; I'd argue those languages are in many cases easier to learn (if harder to use) than modern languages. The hard part is the generations of legacy kruft that's accumulated in them. It's grandma's attic, and granny was a pack rat.

[Advertisement] Keep all your packages and Docker containers in one place, scan for vulnerabilities, and control who can access different feeds. ProGet installs in minutes and has a powerful free version with a lot of great features that you can upgrade when ready.Learn more.

User inputs are frequently incorrect, which is why we validate them. So, for example, if the user is allowed to enter an "asset ID" to perform some operation on it, we should verify that the asset ID exists before actually doing the operation.

Someone working with Capybara James almost got there. Almost.

private boolean isAssetIdMatching(String requestedAssetId, String databaseAssetId) { return (requestedAssetId + "").equals(databaseAssetId + ""); }

This Java code checks if the requestedAssetId, provided by the user, matches a databaseAssetId, fetched from the database. I don't fully understand how we get to this particular function. How is the databaseAssetId fetched? If the fetch were successful, how could it not match? I fear they may do this in a loop across all of the asset IDs in the database until they find a match, but I don't know that for sure, but the naming conventions hint at a WTF.

The weird thing here, though, is the choice to concatenate an empty string to every value. There's no logical reason to do this. It certainly won't change the equality check. I strongly suspect that the goal here was to protect against null values, but it doesn't work that way in Java. If the string variables are null, this will just throw an exception when you try and concatenate.

I strongly suspect the developer was more confident in JavaScript, where this pattern "works".

I don't understand why or how this function got here. I'm not the only one. James writes:

No clue what the original developers were intending with this. It sure was a shocker when we inherited a ton of code like this.

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