When faced with an information system lacking sufficient richness to permit its users to express all of the necessary data states, human beings will innovate. In other words, they will find creative ways to bend the system to their will, usually (but not always) inconsequentially.

In the early days of information systems, even before electronic computers, we found users choosing to insert various out-of-bounds values into data fields to represent states such as "I don't know the true value for this item" or "It is impossible accurately state the true value of this item because of faulty constraint being applied to the input mechanism" or other such notions.

This practice carried on into the computing age, so that now, numeric fields will often contain values of 9999 or 99999999. Taxpayer numbers will be listed as 000-00-0000 or any other repetition of the same digit or simple sequences. Requirements to enter names collected John Does. Now we also see a fair share of Disney characters.

Programmers then try to make their systems idiot-proof, with the obvious and entirely predictable results.

The mere fact that these inventions exist at all is entirely due to the ommission of mechanisms for the metacommentary that we all know perfectly well is sometimes necessary. But rather than provide those, it's easier to wave our hands and pretend that these unwanted states won't exist, can be ignored, can be glossed over. "Relax" they'll tell you. "It probably won't ever happen." "If it does happen, it won't matter." "Don't lose your head over it."

The Beast in Black certainly isn't inclined to cover up an errant sentinel. "For that price, it had better be a genuine Louis XVI pillow from 21-January-1793." A La Lanterne!

 

Daniel D. doubled up on Error'ds for us. "Do you need the error details? Yes, please."

 

And again with an alert notification oopsie. "Google Analytics 4 never stops surprising us any given day with how bugged it is. I call it an "Exclamation point undefined". You want more info? Just Google it... Oh wait." I do appreciate knowing who is responsible for the various bodges we are sent. Thank you, Daniel.

 

"Dark pattern or dumb pattern?" wonders an anonymous reader. I don't think it's very dark.

 

Finally, Ian Campbell found a data error that doesn't look like an intentional sentinel. But I'm not sure what this number represents. It is not an integral power of 2. Says Ian, "SendGrid has a pretty good free plan now with a daily limit of nine quadrillion seven trillion one hundred ninety-nine billion two hundred fifty-four million seven hundred forty thousand nine hundred ninety-two."

 

[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!

You know what definitely never changes? Shipping prices. Famously static, despite all economic conditions and the same across all shipping providers. It doesn't matter where you're shipping from, or to, you know exactly what the price will be to ship that package at all times.

Wait, what? You don't think that's true? It must be true, because Chris sent us this function, which calculates shipping prices, and it couldn't be wrong, could it?

public double getShippingCharge(String shippingType, bool saturday, double subTot) { double shCharge = 0.00; if(shippingType.Equals("Ground")) { if(subTot <= 29.99 && subTot > 0) { shCharge = 4.95; } else if(subTot <= 99.99 && subTot > 29.99) { shCharge = 7.95; } else if(subTot <= 299.99 && subTot > 99.99) { shCharge = 9.95; } else if(subTot > 299.99) { shCharge = subTot * .05; } } else if(shippingType.Equals("Two-Day")) { if(subTot <= 29.99 && subTot > 0) { shCharge = 14.95; } else if(subTot <= 99.99 && subTot > 29.99) { shCharge = 19.95; } else if(subTot <= 299.99 && subTot > 99.99) { shCharge = 29.95; } else if(subTot > 299.99) { shCharge = subTot * .10; } } else if(shippingType.Equals("Next Day")) { if(subTot <= 29.99 && subTot > 0) { shCharge = 24.95; } else if(subTot <= 99.99 && subTot > 29.99) { shCharge = 34.95; } else if(subTot <= 299.99 && subTot > 99.99) { shCharge = 44.95; } else if(subTot > 299.99) { shCharge = subTot * .15; } } else if(shippingType.Equals("Next Day a.m.")) { if(subTot <= 29.99 && subTot > 0) { shCharge = 29.95; } else if(subTot <= 99.99 && subTot > 29.99) { shCharge = 39.95; } else if(subTot <= 299.99 && subTot > 99.99) { shCharge = 49.95; } else if(subTot > 299.99) { shCharge = subTot * .20; } } return shCharge; }

Next you're going to tell me that passing the shipping types around as stringly typed data instead of enums is a mistake, too!

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

Singletons is arguably the easiest to understand design pattern, and thus, one of the most frequently implemented design patterns, even- especially- when it isn't necessary. Its simplicity is its weakness.

Bartłomiej inherited some code which implemented this pattern many, many times. None of them worked quite correctly, and all of them tried to create a singleton a different way.

For example, this one:

public class SystemMemorySettings { private static SystemMemorySettings _instance; public SystemMemorySettings() { if (_instance == null) { _instance = this; } } public static SystemMemorySettings GetInstance() { return _instance; } public void DoSomething() { ... // (this must only be done for singleton instance - not for working copy) if (this != _instance) { return; } ... } }

The only thing they got correct was the static method which returns an instance, but everything else is wrong. They construct the instance in the constructor, meaning this isn't actually a singleton, since you can construct it multiple times. Each new construction replaces the shared instance with a new one.

The real "magic" here, however, is the DoSomething, which checks if the currently active instance is also the most recently constructed instance. If it isn't, this function just fails silently and does nothing.

A common critique of singletons is that they're simply "global variables with extra steps," but this doesn't even succeed at that- it's just a failure, top to bottom.

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

Honestly, I don't know what to say about this code sent to us by Austin, beyond "I think somebody was very confused".

string text; text = ""; // snip box.Text = text; text = ""; text = XMLUtil.SanitizeXmlString(text);

This feels like it goes beyond the usual cruft and confusion that comes with code evolving without ever really being thought about, and ends up in some space outside of meaning. It's all empty strings, signifying nothing, but we've sanitized it.

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

Do you know what I had forgotten until this morning? That VBScript (and thus, older versions of Visual Basic) don't require you to use parentheses when calling a function. Foo 5 and Foo(5) are the same thing.

Of course, why would I remember that? I thankfully haven't touched any of those languages since about… 2012. Which is actually a horrifyingly short time ago, back when I supported classic ASP web apps. Even when I did, I always used parentheses because I wanted my code to be something close to readable.

Classic ASP, there's a WTF for you. All the joy of the way PHP mixes markup and code into a single document, but with an arguably worse and weirder language.

Which finally, brings us to Josh's code. Josh worked for a traveling exhibition company, and that company had an entirely homebrewed CMS written in classic ASP. Here's a few hundred lines out of their navigation menu.

<ul class=menuMain> <% if menu = "1" then Response.Write "<li class='activ'><b></b><i></i><a href='/home.asp' title='Home'>Home</a></li>" else Response.Write "<li><a href='/home.asp' title='Home'>Home</a></li>" end if if menu = "2" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/about_wc_homepage.asp' title='About World Challenge'>About us</a></li>" else Response.Write "<li><a href='/expeditions/about_wc_homepage.asp' title='About World Challenge'>About us</a></li>" end if if menu = "3" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/book-a-school-expedition.asp' title='How to book'>How to book</a></li>" else Response.Write "<li><a href='/expeditions/book-a-school-expedition.asp' title='How to book'>How to book</a></li>" end if if menu = "4" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/expeditions_home.asp' title='Expeditions'>Expeditions</a></li>" else Response.Write "<li><a href='/expeditions/expeditions_home.asp' title='Expeditions'>Expeditions</a></li>" end if if menu = "5" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/safety_home.asp' title='Safety'>Safety</a></li>" else Response.Write "<li><a href='/expeditions/safety_home.asp' title='Safety'>Safety</a></li>" end if if menu = "6" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/mm_what_is_mm.asp' title='Fundraising support'>Fundraising</a></li>" else Response.Write "<li><a href='/expeditions/mm_what_is_mm.asp' title='Fundraising support'>Fundraising</a></li>" end if if menu = "7" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/careers_home.asp' title='Work for us'>Work for us</a></li>" else Response.Write "<li><a href='/expeditions/careers_home.asp' title='Work for us'>Work for us</a></li>" end if if menu = "8" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/contact_us_home.asp' title='Contact us'>Contact us</a></li>" else Response.Write "<li><a href='/expeditions/contact_us_home.asp' title='Contact us'>Contact us</a></li>" end if Response.Write "</ul>" Response.Write "<ul class='menuSub'>" if menu = "1" then end if if menu = "2" then if submenu = "1" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/about_wc_who_we_are.asp' title='Who we are'>Who we are</a></li>" else Response.Write "<li><a href='/expeditions/about_wc_who_we_are.asp'title='Who we are'>Who we are</a></li>" end if if submenu = "2" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/world_challenge_CSR.asp' title='CSR'>CSR</a></li>" else Response.Write "<li><a href='/expeditions/world_challenge_CSR.asp' title='CSR'>CSR</a></li>" end if if submenu = "3" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/World-Challenge-Accreditation.asp' title='Partners and accreditation'>Partners and accreditation</a></li>" else Response.Write "<li><a href='/expeditions/World-Challenge-Accreditation.asp' title='Partners and accreditation'>Partners and accreditation</a></li>" end if if submenu = "4" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/curriculum-links.asp' title='Curriculum links'>Curriculum links</a></li>" else Response.Write "<li><a href='/expeditions/curriculum-links.asp' title='Curriculum links'>Curriculum links</a></li>" end if if submenu = "5" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/expedition_advice.asp' title='Expedition advice'>Expedition advice</a></li>" else Response.Write "<li><a href='/expeditions/expedition_advice.asp' title='Expedition advice'>Expedition advice</a></li>" end if if submenu = "6" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/about_wc_press_and_publications.asp' title='Press resources'>Press resources</a></li>" else Response.Write "<li><a href='/expeditions/about_wc_press_and_publications.asp' title='Press resources'>Press resources</a></li>" end if end if if menu = "3" then Response.Write "<li></li>" end if if menu = "4" then if submenu = "1" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/exped_lh_dest_ca.asp' title='Central & North America'>Central and North America</a></li>" else Response.Write "<li><a href='/expeditions/exped_lh_dest_ca.asp' title='Central and North America'>Central and North America</a></li>" end if if submenu = "2" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/exped_lh_dest_sa.asp' title='South America'>South America</a></li>" else Response.Write "<li><a href='/expeditions/exped_lh_dest_sa.asp' title='South America'>South America</a></li>" end if if submenu = "3" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/exped_lh_dest_sea.asp' title='South East Asia'>South East Asia</a></li>" else Response.Write "<li><a href='/expeditions/exped_lh_dest_sea.asp' title='South East Asia'>South East Asia</a></li>" end if if submenu = "4" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/exped_lh_dest_asia.asp' title='Asia'>Asia</a></li>" else Response.Write "<li><a href='/expeditions/exped_lh_dest_asia.asp' title='Asia'>Asia</a></li>" end if if submenu = "5" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/exped_lh_dest_africa.asp' title='Africa'>Africa</a></li>" else Response.Write "<li><a href='/expeditions/exped_lh_dest_africa.asp' title='Africa'>Africa</a></li>" end if if submenu = "6" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/europe_school_expeditions.asp' title='Europe'>Europe</a></li>" else Response.Write "<li><a href='/expeditions/europe_school_expeditions.asp' title='Europe'>Europe</a></li>" end if if submenu = "7" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/community-projects.asp' title='Community projects'>Community projects</a></li>" else Response.Write "<li><a href='/expeditions/community-projects.asp' title='Community projects'>Community projects</a></li>" end if if submenu = "8" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/exped_indiv_home.asp' title='Independent'>Independent</a></li>" else Response.Write "<li><a href='/expeditions/exped_indiv_home.asp' title='Independent'>Independent</a></li>" end if end if if menu = "5" then if submenu = "1" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/safe-people.asp' title='Safe People'>Safe people</a></li>" else Response.Write "<li><a href='/expeditions/safe-people.asp' title='Safe People'>Safe people</a></li>" end if if submenu = "2" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/safe-place.asp' title='Safe places'>Safe places</a></li>" else Response.Write "<li><a href='/expeditions/safe-place.asp' title='Safe places'>Safe places</a></li>" end if if submenu = "3" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/safe-policies-practises.asp' title='Safe practices and policies'>Safe practices and policies</a></li>" else Response.Write "<li><a href='/expeditions/safe-policies-practises.asp' title='Safe practices and policies'>Safe practices and policies</a></li>" end if if submenu = "4" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/safe-resources.asp' title='Safe Resources'>Safe resources</a></li>" else Response.Write "<li><a href='/expeditions/safe-resources.asp' title='Safe Resources'>Safe resources</a></li>" end if if submenu = "5" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/safety_ops_centre.asp' title='Operations Centre'>Operations Centre</a></li>" else Response.Write "<li><a href='/expeditions/safety_ops_centre.asp' title='Operations Centre'>Operations Centre</a></li>" end if if submenu = "6" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/travel_safety_course.asp' title='Travelsafe course'>Travelsafe course</a></li>" else Response.Write "<li><a href='/expeditions/travel_safety_course.asp' title='Travelsafe course'>Travelsafe course</a></li>" end if end if if menu = "6" then ' if submenu = "1" then ' Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/fundraising-team.asp' title='Fundraising team'>Fundraising team</a></li>" ' else ' Response.Write "<li><a href='/expeditions/fundraising-team.asp' title='Fundraising team'>Fundraising team</a></li>" ' end if if submenu = "2" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/mm_ideas.asp' title='Fundraising ideas'>Fundraising ideas</a></li>" else Response.Write "<li><a href='/expeditions/mm_ideas.asp' title='Fundraising ideas'>Fundraising ideas</a></li>" end if if submenu = "3" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/about_wc_events_challenger_events.asp' title='Fundraising events'>Fundraising events</a></li>" else Response.Write "<li><a href='/expeditions/about_wc_events_challenger_events.asp' title='Fundraising events'>Fundraising events</a></li>" end if end if if menu = "7" then if submenu = "1" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/careers_leader_ops_overseas.asp' title='Lead an expedition'>Lead an expedition</a></li>" else Response.Write "<li><a href='/expeditions/careers_leader_ops_overseas.asp' title='Lead an expedition'>Lead an expedition</a></li>" end if if submenu = "2" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/permanent_jobs_world_challenge.asp' title='Office based positions'>Office based positions</a></li>" else Response.Write "<li><a href='/expeditions/permanent_jobs_world_challenge.asp' title='Office based positions'>Office based positions</a></li>" end if end if if menu = "8" then if submenu = "1" then Response.Write "<li class='activ'><b></b><i></i><a href='/pages/forms-brochure.asp' title='Request a brochure'>Request a brochure</a></li>" else Response.Write "<li><a href='/pages/forms-brochure.asp' title='Request a brochure'>Request a brochure</a></li>" end if if submenu = "2" then Response.Write "<li class='activ'><b></b><i></i><a rel='external' href='http://f.chtah.com/s/3/2069554126/signup.html' title='Sign up for e-news'>Sign up for e-news</a></li>" else Response.Write "<li><a rel='external' href='http://f.chtah.com/s/3/2069554126/signup.html' title='Sign up for e-news'>Sign up for e-news</a></li>" end if if submenu = "3" then Response.Write "<li class='activ'><b></b><i></i><a href='/expeditions/about_wc_press_and_publications.asp' title='Press resources'>Press resources</a></li>" else Response.Write "<li><a href='/expeditions/about_wc_press_and_publications.asp' title='Press resources'>Press resources</a></li>" end if end if %> </ul>

This renders the whole menu, but based on the selected menu and submenu, it adds an activ class to the HTML elements. Which means that each HTML element is defined here twice, once with and without the CSS class on it. I know folks like to talk about dry code, but this code is SOGGY with repetition. Just absolutely dripping wet with the same thing multiple times. Moist.

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

I never did explain the elusive off-by-one I hinted at, did I? A little meta, perhaps. It is our practice at Error'd to supply five nuggets of joy each week. But in episode previous-plus-one, you actually got six! (Or maybe, depending on how you count them, that's yet another off-by-one. I slay me.) If that doesn't tickle you enough, just wait until you hear what Dave L. brought us. Meanwhile...

"YATZP" scoffed self-styled Foo AKA F. Yet Another Time Zone P*, I guess. Not wrong. According to Herr Aka F., "German TV teletext (yes, we still have it!) botched the DST start (upper right corner). The editors realized it and posted a message stating as much, sent from the 'future' (i.e. correct) time zone."

 

Michael R. wrote in with a thought-provoker. If I'm representing one o'clock as 1:00, two o'clock as 2:00, and so forth, why should zero o'clock be the only time represented with not just one, but TWO leading zeroes? Logically, zero o'clock should be represented simply by :00, right?

 

Meanwhile, (just) Randy points out that somebody failed to pay attention to detail. "Did a full-scroll on Baeldung's members page and saw this. Sometimes, even teachers don't get it right."

 

In case Michael R. is still job-hunting Gary K. has found the perfect position for everyone. That is, assuming the tantalizingly missing Pay Range section conforms to the established pattern. "Does this mean I should put my qualifications in?" he wondered. Run, don't walk.

 

And in what I think is an all-time first for us, Dave L. brings (drum roll) an audio Error'd "I thought you'd like this recording from my Garmin watch giving me turn-by-turn directions: In 280.097 feet turn right. That's two hundred eighty feet and ONE POINT ONE SIX FOUR INCHES. Accuracy to a third of a millimeter!" Don't move your hand!

Alas, your browser does not support this audio.

 

[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.

Sean sends us a one-line function that is a delight, if by delight you mean "horror". You'll be shocked to know it's PHP.

function proget(){foreach($_GET as $k=>$v){if($k=="h"){$GLOBALS["h"]=1;}$p=explode(",",$k);}return($p);} //function to process GET headers

Based on the comment, proget is a shorthand for process_get_parameters. Which is sort of what it does. Sort of.

Let's go through this. We iterate across our $_GET parameters using $k for the key, $v for the value, but we never reference the value so forget it exists. We're iterating across every key. The first thing we check is if a key "h" exists. We don't look at its value, we just check if it exists, and if it does, we set a global variable. And this, right here, is enough for this to be a WTF. The logic of "set a global variable based on the existence of a query parameter regardless of the value of the query parameter" is… a lot. But then, somehow, this actually gets more out there.

We explode the key on commas (explode being PHP's much cooler name for split), which implies… our keys may be lists of values? Which I feel like is an example of someone not understanding what a "key" is. But worse than that, we just do this for every key, and return the results of performing that operation on the last key. Which means that if this function is doing anything at all, it's entirely dependent on the order of the keys. Which, PHP does order the keys by the order they're added, which I take to mean that if the URL has query params like ?foo=1&h=0&a,b,c,d=wtf. Or, if we're being picky about encoding, ?foo=1&h=0&a%2Cb%2Cc%2Cd=wtf. The only good news here is that PHP handles the encoding/decoding for you, so the explode will work as expected.

This is the kind of bad code that leaves me with lots of questions, and I'm not sure I want any of the answers. How did this happen, and why are questions best left unanswered, because I think the answers might cause more harm.

[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!

Today's anonymous submitter worked for a "large, US-based, e-commerce company." This particular company was, some time back, looking to save money, and like so many companies do, that meant hiring offshore contractors.

Now, I want to stress, there's certainly nothing magical about national borders which turns software engineers into incompetents. The reality is simply that contractors never have their client's best interests at heart; they only want to be good enough to complete their contract. This gets multiplied by the contracting firm's desire to maximize their profits by keeping their contractors as booked as possible. And it gets further multiplied by the remoteness and siloing of the interaction, especially across timezones. Often, the customer sends out requirements, and three months later gets a finished feature, with no more contact than that- and it never goes well.

All that said, let's look at some SQL Server code. It's long, so we'll take it in chunks.

-- =============================================================================== -- Author : Ignacius Ignoramus -- Create date: 04-12-2020 -- Description: SP of Getting Discrepancy of Allocation Reconciliation Snapshot -- ===============================================================================

That the comment reinforces that this is an "SP", aka stored procedure, is already not my favorite thing to see. The description is certainly made up of words, and I think I get the gist.

ALTER PROCEDURE [dbo].[Discrepency] ( @startDate DATETIME, @endDate DATETIME ) AS BEGIN

Nothing really to see here; it's easy to see that we're going to run a query for a date range. That's fine and common.

DECLARE @tblReturn TABLE ( intOrderItemId INT )

Hmm. T-SQL lets you define table variables, which are exactly what they sound like. It's a local variable in this procedure, that acts like a table. You can insert/update/delete/query it. The vague name is a little sketch, and the fact that it holds only one field also makes me go "hmmm", but this isn't bad.

DECLARE @tblReturn1 TABLE ( intOrderItemId INT )

Uh oh.

DECLARE @tblReturn2 TABLE ( intOrderItemId INT )

Oh no.

DECLARE @tblReturn3 TABLE ( intOrderItemId INT )

Oh no no no.

DECLARE @tblReturn4 TABLE ( intOrderItemId INT )

This doesn't bode well.

So they've declared five variables called tblReturn, that all hold the same data structure.

What happens next? This next block is gonna be long.

INSERT INTO @tblReturn --(intOrderItemId) VALUES (@_ordersToBeAllocated) /* OrderItemsPlaced */ select intOrderItemId from CompanyDatabase..Orders o inner join CompanyDatabase..OrderItems oi on oi.intOrderId = o.intOrderId where o.dtmTimeStamp between @startDate and @endDate AND intOrderItemId Not In ( /* _itemsOnBackorder */ select intOrderItemId from CompanyDatabase..OrderItems oi inner join CompanyDatabase..Orders o on o.intOrderId = oi.intOrderId where o.dtmTimeStamp between @startDate and @endDate and oi.strstatus='backordered' ) AND intOrderItemId Not In ( /* _itemsOnHold */ select intOrderItemId from CompanyDatabase..OrderItems oi inner join CompanyDatabase..Orders o on o.intOrderId = oi.intOrderId where o.dtmTimeStamp between @startDate and @endDate and o.strstatus='ONHOLD' and oi.strStatus <> 'BACKORDERED' ) AND intOrderItemId Not In ( /* _itemsOnReview */ select intOrderItemId from CompanyDatabase..OrderItems oi inner join CompanyDatabase..Orders o on o.intOrderId = oi.intOrderId where o.dtmTimeStamp between @startDate and @endDate and o.strstatus='REVIEW' and oi.strStatus <> 'BACKORDERED' ) AND intOrderItemId Not In ( /*_itemsOnPending*/ select intOrderItemId from CompanyDatabase..OrderItems oi inner join CompanyDatabase..Orders o on o.intOrderId = oi.intOrderId where o.dtmTimeStamp between @startDate and @endDate and o.strstatus='PENDING' and oi.strStatus <> 'BACKORDERED' ) AND intOrderItemId Not In ( /*_itemsCancelled */ select intOrderItemId from CompanyDatabase..OrderItems oi inner join CompanyDatabase..Orders o on o.intOrderId = oi.intOrderId where o.dtmTimeStamp between @startDate and @endDate and oi.strstatus='CANCELLED' )

We insert into @tblReturn the result of a query, and this query relies heavily on using a big pile of subqueries to decide if a record should be included in the output- but these subqueries all query the same tables as the root query. I'm fairly certain this could be a simple join with a pretty readable where clause, but I'm also not going to sit here and rewrite it right now, we've got a lot more query to look at.

INSERT INTO @tblReturn1 /* _backOrderItemsReleased */ select intOrderItemId from CompanyDatabase..OrderItems oi inner join CompanyDatabase..orders o on o.intorderid = oi.intorderid where oi.intOrderItemid in ( select intRecordID from CompanyDatabase..StatusChangeLog where strRecordType = 'OrderItem' and strOldStatus in ('BACKORDERED') and strNewStatus in ('NEW', 'RECYCLED') and dtmTimeStamp between @startDate and @endDate ) and o.dtmTimeStamp < @startDate UNION ( /*_pendingHoldItemsReleased*/ select intOrderItemId from CompanyDatabase..OrderItems oi inner join CompanyDatabase..orders o on o.intorderid = oi.intorderid where oi.intOrderID in ( select intRecordID from CompanyDatabase..StatusChangeLog where strRecordType = 'Order' and strOldStatus in ('REVIEW', 'ONHOLD', 'PENDING') and strNewStatus in ('NEW', 'PROCESSING') and dtmTimeStamp between @startDate and @endDate ) and o.dtmTimeStamp < @startDate ) UNION /* _reallocationsowingtonostock */ ( select oi.intOrderItemID from CompanyDatabase.dbo.StatusChangeLog inner join CompanyDatabase.dbo.OrderItems oi on oi.intOrderItemID = CompanyDatabase.dbo.StatusChangeLog.intRecordID inner join CompanyDatabase.dbo.Orders o on o.intOrderId = oi.intOrderId where strOldStatus = 'RECYCLED' and strNewStatus = 'ALLOCATED' and CompanyDatabase.dbo.StatusChangeLog.dtmTimestamp > @endDate and strRecordType = 'OrderItem' and intRecordId in ( select intRecordId from CompanyDatabase.dbo.StatusChangeLog where strOldStatus = 'ALLOCATED' and strNewStatus = 'RECYCLED' and strRecordType = 'OrderItem' and CompanyDatabase.dbo.StatusChangeLog.dtmTimestamp between @startDate and @endDate ) )

Okay, just some unions with more subquery filtering. More of the same. It's the next one that makes this special.

INSERT INTO @tblReturn2 SELECT intOrderItemId FROM @tblReturn UNION SELECT intOrderItemId FROM @tblReturn1

Ah, here's the stuff. This is just bonkers. If the goal is to combine the results of these queries into a single table, you could just insert into one table the whole time.

But we know that there are 5 of these tables, so why are we only going through the first two to combine them at this point?

INSERT INTO @tblReturn3 /* _factoryAllocation*/ select oi.intOrderItemId from CompanyDatabase..Shipments s inner join CompanyDatabase..ShipmentItems si on si.intShipmentID = s.intShipmentID inner join Common.CompanyDatabase.Stores stores on stores.intStoreID = s.intLocationID inner join CompanyDatabase..OrderItems oi on oi.intOrderItemId = si.intOrderItemId inner join CompanyDatabase..Orders o on o.intOrderId = s.intOrderId where s.dtmTimestamp >= @endDate and stores.strLocationType = 'FACTORY' UNION ( /*_storeAllocations*/ select oi.intOrderItemId from CompanyDatabase..Shipments s inner join CompanyDatabase..ShipmentItems si on si.intShipmentID = s.intShipmentID inner join Common.CompanyDatabase.Stores stores on stores.intStoreID = s.intLocationID inner join CompanyDatabase..OrderItems oi on oi.intOrderItemId = si.intOrderItemId inner join CompanyDatabase..Orders o on o.intOrderId = s.intOrderId where s.dtmTimestamp >= @endDate and stores.strLocationType <> 'FACTORY' ) UNION ( /* _ordersWithAllocationProblems */ select oi.intOrderItemId from CompanyDatabase.dbo.StatusChangeLog inner join CompanyDatabase.dbo.OrderItems oi on oi.intOrderItemID = CompanyDatabase.dbo.StatusChangeLog.intRecordID inner join CompanyDatabase.dbo.Orders o on o.intOrderId = oi.intOrderId where strRecordType = 'orderitem' and strNewStatus = 'PROBLEM' and strOldStatus = 'NEW' and CompanyDatabase.dbo.StatusChangeLog.dtmTimestamp > @endDate and o.dtmTimestamp < @endDate )

Okay, @tblReturn3 is more of the same. Nothing more to really add.

INSERT INTO @tblReturn4 SELECT intOrderItemId FROM @tblReturn2 WHERE intOrderItemId NOT IN(SELECT intOrderItemId FROM @tblReturn3 )

Ooh, but here we see something a bit different- we're taking the set difference between @tblReturn2 and @tblReturn3. This would almost make sense if there weren't already set operations in T-SQL which would handle all of this.

Which brings us, finally, to the last query in the whole thing:

SELECT o.intOrderId ,oi.intOrderItemId ,o.dtmDate ,oi.strDescription ,o.strFirstName + o.strLastName AS 'Name' ,o.strEmail ,o.strBillingCountry ,o.strShippingCountry FROM CompanyDatabase.dbo.OrderItems oi INNER JOIN CompanyDatabase.dbo.Orders o on o.intOrderId = oi.intOrderId WHERE oi.intOrderItemId IN (SELECT intOrderItemId FROM @tblReturn4) END

At the end of all this, I've determined a few things.

First, the developer responsible didn't understand table variables. Second,they definitely didn't understand joins. Third, they had no sense of the overall workflow of this query and just sorta fumbled through until they got results that the client said were okay.

And somehow, this pile of trash made it through a code review by internal architects and got deployed to production, where it promptly became the worst performing query in their application. Correction: the worst performing query thus far.

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

Many years ago, JP joined a Ruby project. This was in the heyday of Ruby, when every startup on Earth was using it, and if you weren't building your app on Rails, were you even building an app?

Now, Ruby offers a lot of flexibility. One might argue that it offers too much flexibility, especially insofar as it permits "monkey patching": you can always add new methods to an existing class, if you want. Regardless of the technical details, JP and the team saw that massive flexibility and said, "Yes, we should use that. All of it!"

As these stories usually go, that was fine- for awhile. Then one day, a test started failing because a class name wasn't defined. That was already odd, but what was even odder is that when they searched through the code, that class name wasn't actually used anywhere. So yes, there was definitely no class with that name, but also, there was no line of code that was trying to instantiate that class. So where was the problem?

def controller_class(name) "#{settings.app_name.camelize}::Controllers".constantize.const_get("#{name.to_s.camelize}") end def model_class(name) "#{settings.app_name.camelize}".constantize.const_get("#{name.to_s.camelize}") end def resource_class(name) "#{settings.app_name.camelize}Client".constantize.const_get("#{name.to_s.camelize}") end

It happened because they were dynamically constructing the class names from a settings field. And not just in this handful of lines- this pattern occurred all over the codebase. There were other places where it referenced a different settings field, and they just hadn't encountered the bug yet, but knew that it was only a matter of time before changing a settings file was going to break more functionality in the application.

They wisely rewrote these sections to not reference the settings, and dubbed the pattern the "Caramelize Pattern". They added that to their coding standards as a thing to avoid, and learned a valuable lesson about how languages provide footguns.

Since today's April Fool's Day, consider the prank the fact that everyone learned their lesson and corrected their mistakes. I suppose that has to happen at least sometimes.

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

Legacy systems are hard to change, and even harder to eliminate. You can't simply do nothing though; as technology and user expectations change, you need to find ways to modernize and adapt the legacy system.

That's what happened to Alicia's team. They had a gigantic, spaghetti-coded, monolithic application that was well past drinking age and had a front-end to match. Someone decided that they couldn't touch the complex business logic, but what they could do was replace the frontend code by creating an adapter service; the front end would call into this adapter, and the adapter would execute the appropriate methods in the backend.

Some clever coder named this "Backend for Frontend" or "BFF".

It was not anyone's BFF. For starters, this system didn't actually allow you to just connect a UI to the backend. No, that'd be too easy. This system was actually a UI generator.

The way this works is that you feed it a schema file, written in JSON. This file specifies what input elements you want, some hints for layout, what validation you want the UI to perform, and even what CSS classes you want. Then you compile this as part of a gigantic .NET application, and deploy it, and then you can see your new UI.

No one likes using it. No one is happy that it exists. Everyone wishes that they could just write frontends like normal people, and not use this awkward schema language.

All that is to say, when Alicia's co-worker stood up shortly before lunch, said, "I'm taking off the rest of the day, BFF has broken me," it wasn't particularly shocking to hear- or even the first time that'd happened.

Alicia, not heeding the warning inherent in that statement, immediately tracked down that dev's last work, and tried to understand what had been so painful.

"minValue": 1900, "maxValue": 99,

This, of course, had to be a bug. Didn't it? How could the maxValue be lower than the minValue?

Let's look at the surrounding context.

{ "type": "eventValueBetweenValuesValidator", "eventType": "CalendarYear", "minValue": 1900, "maxValue": 99, "isCalendarBasedMaxValue": true, "message": "CalendarYear must be between {% raw %}{{minValue}}{% endraw %} and {% raw %}{{maxValue}}{% endraw %}." }

I think this should make it perfectly clear what's happening. Oh, it doesn't? Look at the isCalendarBasedMaxValue field. It's true. There, that should explain everything. No, it doesn't? You're just more confused?

The isCalendarBasedMaxValue says that the maxValue field should not be treated as a literal value, but instead, is the number of years in the future relative to the current year which are considered valid. This schema definition says "accept all years between 1900 and 2124 (at the time of this writing)." Next year, that top value goes up to 2125. Then 2126. And so on.

As features go, it's not a terrible feature. But the implementation of the feature is incredibly counter-intuitive. At the end of the day, this is just bad naming: (ab)using min/max to do something that isn't really a min/max validation is the big issue here.

Alicia writes:

I couldn't come up with something more counterintuitive if I tried.

Oh, don't sell yourself short, Alicia. I'm sure you could write something far, far worse if you tried. The key thing here is that clearly, nobody tried- they just sorta let things happen and definitely didn't think too hard about it.

[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.

We got an unusual rash of submissions at Error'd this week. Here are five reasonably good ones chosen not exactly at random. For those few (everyone) who didn't catch the off-by-one from last week's batch, there's the clue.

"Gotta CAPTCHA 'Em All," puns Alex G. "So do I select them all?" he wondered. I think the correct answer is null.

 

"What does a null eat?" wondered B.J.H , "and is one null invited or five?". The first question is easily answered. NaaN, of course. Probably garlic. I would expect B.J. to already know the eating habits of a long-standing companion, so I am guessing that the whole family is not meant to tag along. Stick with just the one.

 

Planespotter Rick R. caught this one at the airport. "Watching my daughter's flight from New York and got surprised by Boeing's new supersonic 737 having already arrived in DFW," he observed. I'm not quite sure what went wrong. It's not the most obvious time zone mistake I can imagine, but I'm pretty sure the cure is the same: all times displayed in any context that is not purely restricted to a single location (and short time frame) should explicitly include the relevant timezone.

 

Rob H. figures "From my day job's MECM Software Center. It appears that autocorrect has miscalculated, because the internet cannot be calculated." The internet is -1.

 

Ending this week on a note of hope, global warrior Stewart may have just saved the planet. "Climate change is solved. We just need to replicate the 19 March performance of my new solar panels." Or perhaps I miscalculated.

 

[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.

Barry rolled into work at 8:30AM to see the project manager waiting at the door, wringing her hands and sweating. She paced a bit while Barry badged in, and then immediately explained the issue:

Today was a major release of their new features. This wasn't just a mere software change; the new release was tied to major changes to a new product line- actual widgets rolling off an assembly line right now. And those changes didn't work.

"I thought we tested this," Barry said.

"We did! And Stu called in sick today!"

Stu was the senior developer on the project, who had written most of the new code.

"I talked to him for a few minutes, and he's convinced it's a data issue. Something in the metadata or something?"

"I'll take a look," Barry said.

He skipped grabbing a coffee from the carafe and dove straight in.

Prior to the recent project, the code had looked something like this:

if (IsProduct1(_productId)) _programId = 1; elseif (IsProduct2(_productId)) _programId = 2; elseif (IsProduct3(_productId)) _programId = 3;

Part of the project, however, was about changing the workflow for "Product 3". So Stu had written this code:

if (IsProduct1(_productId)) _programId = 1; else if (IsProduct2(_productId)) _programId = 2; else if (IsProduct3(_productId)) _programId = 3; DoSomethingProductId3Specific1(); DoSomethingProductId3Specific2(); DoSomethingProductId3Specific3();

Since this is C# and not Python, it took Barry all of 5 seconds to spot this and figure out what the problem was and fix it:

if (IsProduct1(_productId)) { _programId = 1; } else if (IsProduct2(_productId)) { _programId = 2; } else if (IsProduct3(_productId)) { _programId = 3; DoSomethingProductId3Specific1(); DoSomethingProductId3Specific2(); DoSomethingProductId3Specific3(); }

This brings us to about 8:32. Now, given the problems, Barry wasn't about to just push this change- in addition to running pipeline tests (and writing tests that Stu clearly hadn't), he pinged the head of QA to get a tester on this fix ASAP. Everyone worked quickly, and that meant by 9:30 the fix was considered good and ready to be merged in and pushed to production. Sometime in there, while waiting for a pipeline to complete, Barry managed to grab a cup of coffee to wake himself up.

While Barry was busy with that, Stu had decided that he wasn't feeling that sick after all, and had rolled into the office around 9:00. Which meant that just as Barry was about to push the button to run the release pipeline, an "URGENT" email came in from Stu.

"Hey, everybody, I fixed that bug. Can we get this released ASAP?"

Barry went ahead and released the version that he'd already tested, but out of morbid curiosity, went and checked Stu's fix.

if (IsProduct1(_productId)) _programId = 1; else if (IsProduct2(_productId)) _programId = 2; else if (IsProduct3(_productId)) { _programId = 3; } if (IsProduct3(_productId)) { DoSomethingProductId3Specific1(); DoSomethingProductId3Specific2(); DoSomethingProductId3Specific3(); }

At least this version would have worked, though I'm not sure Stu fully understands what "{}"s mean in C#. Or in most programming languages, if we're being honest.

With Barry's work, the launch went off just a few minutes later than the scheduled time. Since the launch was successful, at the next company "all hands", the leadership team made sure to congratulate the people instrumental in making it happen: that is to say, the lead developer of the project, Stu.

[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.

If you need a unique ID, UUIDs provide a variety of options. It's worth noting that variants 1, 2, and 7 all incorporate a timestamp into the UUID. In the case of variant 7, this has the benefit of making the UUID sortable, which can be convenient in many cases (v1/v2 incorporate a MAC address which means that they're sortable if generated with the same NIC).

I bring this up because Dave inherited some code written by a "guru". Said guru was working before UUIDv7 was a standard, but also didn't have any problems that required sortable UUIDs, and thus had no real reason to use timestamp based UUIDs. They just needed some random identifier and, despite using C#, didn't use the UUID functions built in to the framework. No, they instead did this:

string uniqueID = String.Format("{0:d9}", (DateTime.UtcNow.Ticks / 10) % 1000000000);

A Tick is 100 nanoseconds. We divide that by ten, mod by a billion, and then call that our unique identifier.

This is, as you might guess, not unique. First there's the possibility of timestamp collisions: generating two of these too close together in time would collide. Second, the math is just complete nonsense. We divide Ticks by ten (converting hundreds of nanoseconds into thousands of nanoseconds), then we mod by a billion. So every thousand seconds we loop and have a risk of collision again?

Maybe, maybe, these are short-lived IDs and a thousand seconds is plenty of time. But even if that's true, none of this is a good way to do that.

I suppose the saving grace is they use UtcNow and not Now, thus avoiding situations where collisions also happen because of time zones?

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

Today's anonymous submitter has managed to find a way to do date formatting wrong that I don't think I've seen yet. That's always remarkable. Like most such bad code, it checks string lengths and then adds a leading zero, if needed. It's not surprising, but again, it's all in the details:

// convert date string to yyyy/MM/DD return dtmValue.Year + "-" + ((dtmValue.Month.ToString().Length == 1)? ("0" + dtmValue.Month.ToString()): dtmValue.Month.ToString()) + "-" + ((dtmValue.Day.ToString().Length == 1)? ("0" + dtmValue.Day.ToString()): dtmValue.Day.ToString());

This is only one line, but it has it all, doesn't it. First, we've got good ol' Hungarian notation, which conveys no useful information here. We've got a comment which tells us the code outputs /es, but the code actually outputs -. We've got ternaries that are definitely not helping readability here, plus repeated calls to ToString() instead of maybe just storing the result in a variable.

And, for the record, dtmValue.ToString("yyyy-MM-dd") would have done the correct thing.

.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.

At one point, someone noticed that some financial transactions weren't summing up correctly in the C# application Nancy supported. It didn't require Superman or a Peter Gibbons to figure out why: someone was using floating points for handling dollar amounts.

That kicked off a big refactoring project to replace the usage of double types with decimal types. Everything seemed to go well, at least until there was a network hiccup and the application couldn't connect to the database. Let's see if you can figure out what happened:

MessageBox.Show("Please decimal check the connection details. Also check firewall settings (port 1433) and network connectivity.");

What a clbuttic mistake.

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

Today we have a whole batch of category errors, picked out from the rash of submissions and a few that have been festering on the shelf. Just for fun, I threw in an ironic off-by-some meta-error. See if you can spot it.

Adam R. "I'm looking for hotel rooms for the 2026 Winter Olympics in Milan-Cortina. Most hotels haven't opened up reservations yet, except for ridiculously overprice hospitality packages. This search query found NaN facilities available, which equates to one very expensive apartment. I guess one is not a number now?"

 

Intrepid traveler BJH had a tough time at the Intercontinental. I almost feel sympathy. Almost. "I stare at nulls at home all the time so it made me feel comfortable to see them at the hotel when traveling. And what is that 'INTERCONTINENTAL W...' at the top? I may never know!"

 

Hoping to find out, BJ clicked through the mystery menu and discovered... this. But even worse, "There was no exit: Clicking Exit did nothing and neither did any of the buttons on the remote. Since I'd received more entertainment than usual from a hotel screen I just turned everything off."

 

Striking out for some streaming entertainment Dmitry NoLastName was silently stymied by this double-decker from Frontier.com.

 

No streaming needed for Barry M. who can get a full dose of fun from those legacy broadcast channels! Whatever did they do before null null undefined null? "Hey, want to watch TV tonight? NaN."

 

Hah! "That's MISTER Null, to you," declared an anonymous contributor.

 

And finally, another entirely different anonymous contributor clarified that there are apparently NaN cellphone towers in Switzerland. Personally, I'm intrigued by the existence of that one little crumb of English on an otherwise entirely German page.

 

[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.

Jenny had been perfectly happy working on a series of projects for her company, before someone said, "Hey, we need you to build a desktop GUI for an existing API."

The request wasn't the problem, per se. The API, on the other hand, absolutely was.

The application Jenny was working on represented a billing contract for materials consumed at a factory. Essentially, the factory built a bunch of individual parts, and then assembled them into a finished product. They only counted the finished product, but needed to itemize the billing down to not only the raw materials that went into the finished product, the intermediate parts, but also the toilet paper put in the bathrooms. All the costs of operating the factory were derived from the units shipped out.

This meant that the contract itself was a fairly complicated tree structure. Jenny's application was meant to both visualize and allow users to edit that tree structure to update the billing contract in sane, and predictable ways, so that it could be reviewed and approved and when the costs of toilet paper went up, those costs could be accurately passed on to the customer.

Now, all the contract management was already implemented and lived library that itself called back into a database. Jenny just needed to wire it up to a desktop UI. Part of the requirements were that line items in the tree needed to have a special icon displayed next to them under two conditions: if one of their ancestors in the tree had been changed since the last released contract, or if they child was marked as "inherit from parent".

The wrapper library wasn't documented, so Jenny asked the obvious question: "What's the interface for this?"

The library team replied with this:

IModelInheritFromParent : INotifyPropertyChanged { bool InheritFromParent {get; set;} }

"That covers the inheritance field," Jenny said, "but that doesn't tell me if the ancestor has been modified."

"Oh, don't worry," the devs replied, "there's an extension method for that."

public bool GetChangedIndicator(this IModelTypeA);

Extension methods in C# are just a way to use syntactic sugar to "add" methods to a class: IModelTypeA does not have a GetChangedIndicator method, but because of the this keyword, it's an extension method and we can now invoke aInstance.GetChangedIndicator(). It's how many built-in .Net APIs work, but like most forms of syntactic sugar, while it can be good, it usually makes code harder to understand, harder to test, and harder to debug.

But Jenny's main complaint was this: "You can't raise an event or something? I'm going to need to poll?"

"Yes, you're going to need to poll."

Jenny didn't like the idea of polling the (slow) database, so at first, she tried to run the polling in a background thread so it wouldn't block the UI. Unfortunately for her, the library was very much not threadsafe, so that blew up. She ended up needing to poll on the main UI thread, which meant the application would frequently stall while users were working. She did her best to minimize it, but it was impossible to eliminate.

But worse than that, each contract item may implement one of four interfaces, which meant there were four versions of the extension method:

public bool GetChangedIndicator(this IModelTypeA); public bool GetChangedIndicator(this IModelTypeB); public bool GetChangedIndicator(this IModelTypeC); public bool GetChangedIndicator(this IModelTypeD);

To "properly" perform the check, Jenny would have to check which casts were valid for a given item, cast it, and then invoke GetChangedIndicator. It's worth noting that had they just used regular inheritance instead of extension methods, this wouldn't have been necessary at all. Using the "fun" syntactic sugar made the code more complicated for no benefit.

This left Jenny with another question: "What if an item implements more than one of these interfaces? What if the extension methods disagree on if the item is changed?"

"Good question," the team responsible for the library replied. "That should almost never happen."

Jenny quit not long after this.

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

Once upon a time, Ryan's company didn't use a modern logging framework to alert admins when services failed. No, they used everyone's favorite communications format, circa 2005: email. Can't reach the database? Send an email. Unhandled exception? Send an email. Handled exception? Better send an email, just in case. Sometimes they go to admins, sometimes they just go to an inbox used for logging.

Let's look at how that worked.

public void SendEMail(String receivers, String subject, String body) { try { System.Net.Mail.SmtpClient clnt = new System.Net.Mail.SmtpClient(ConfigurationManager.AppSettings["SmtpServer"]); clnt.Send(new System.Net.Mail.MailMessage( ConfigurationManager.AppSettings["Sender"], ConfigurationManager.AppSettings["Receivers"], subject, body)); } catch (Exception ex) { SendEMail( ConfigurationManager.AppSettings["ErrorLogAddress"], "An error has occurred while sending an email", ex.Message + "\n" + ex.StackTrace); } }

They use the Dot Net SmtpClient class to connect to an SMTP server and send emails based on the configuration. So far so good, but what happens when we can't send an email because the email server is down? We'll get an exception, and what do we do with it?

The same thing we do with every other exception: send an email.

Ryan writes:

Strangely enough, I've never heard of the service crashing or hanging. We must have a very good mail server!

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

Alex had the misfortune to work on the kind of application which has forms with gigantic piles of fields, stuffed haphazardly into objects. A single form could easily have fifty or sixty fields for the user to interact with.

That leads to C# code like this:

private static String getPrefix(AV_Suchfilter filter) { String pr = String.Empty; try { int maxLength = 0; if (filter.Angebots_id != null) { maxLength = getmaxLength(maxLength, AV_MessagesTexte.Reportliste_sf_angebotsID.Length); } if (filter.InternesKennzeichen != null) { if (filter.InternesKennzeichen.Trim() != String.Empty) { maxLength = getmaxLength(maxLength, AV_MessagesTexte.Reportliste_sf_internesKennzeichen.Length); } } if (filter.Angebotsverantwortlicher_guid != null) { maxLength = getmaxLength(maxLength, AV_MessagesTexte.Reportliste_sf_angebotsverantwortlicher.Length); } // Do this another 50 times.... // and then .... int counter = 0; while (counter < maxLength) { pr += " "; counter++; } } catch (Exception error) { ErrorForm frm = new ErrorForm(error); frm.ShowDialog(); } return pr; }

The "Do this another 50 times" is doing a lot of heavy lifting in here. What really infuriates me about it, though, which we can see here, is that not all of the fields we're looking at are parameters to this function. And because the function here is static, they're not instance members either. I assume AV_MessagesTexte is basically a global of text labels, which isn't a bad way to manage such a thing, but functions should still take those globals as parameters so you can test them.

I'm kidding, of course. This function has never been tested.

Aside from a gigantic pile of string length comparisons, what does this function actually do? Well, it returns a new string which is a number of spaces exactly equal to the length of the longest string. And the way we build that output string is not only through string concatenation, but the use of a while loop where a for loop makes more sense.

Also, just… why? Why do we need a spaces-only-string the length of another string? Even if we're trying to do some sort of text layout, that seems like a bad way to do whatever it is we're doing, and also if that's the case, why is it called getPrefix? WHY IS OUR PREFIX A STRING OF SPACES THE LENGTH OF OUR FIELD? HOW IS THAT A PREFIX?

I feel like I'm going mad.

But the real star of this horrible mess, in my opinion, is the exception handling. Get an exception? Show the user a form! There's no attempt to decide if or how we could recover from this error, we just annoy the user with it.

Which isn't just unique to this function. Notice the getmaxLength function? It's really a max and it looks like this:

private static int getmaxLength(int old, int current) { int result = old; try { if (current > old) { result = current; } } catch (Exception error) { ErrorForm frm = new ErrorForm(error); frm.ShowDialog(); } return result; }

What's especially delightful here is that this function couldn't possibly throw an exception. And you know what that tells me? This try/catch/form pattern is just their default error handling. They spam this everywhere, in every function, and the tech lead or architect pats themselves on the back for ensuring that the application "never crashes!" all the while annoying the users with messages they can't do anything about.

.comment { border: none; } [Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!

Fresh out of university, Remco accepted a job that allowed him to relocate to a different country. While entering the workforce for the first time, he was also adjusting to a new home and culture, which is probably why the red flags didn't look quite so red.

The trouble had actually begun during his interview. While being questioned about his own abilities, Remco learned about Conglomcorp's healthy financial position, backed by a large list of clients. Everything seemed perfect, but Remco had a bad gut feeling he could neither explain nor shake off. Being young and desperate for a job, he ignored his misgivings and accepted the position. He hadn't yet learned how scarily accurate intuition often proves to be.

The second red flag was run up the mast at orientation. While teaching him about the company's history, one of the senior managers proudly mentioned that Conglomcorp had recently fired 50% of their workforce, and were still doing great. This left Remco feeling more concerned than impressed, but he couldn't reverse course now.

Flag number three waved during onboarding, as Remco began to learn about the Java application he would be helping to develop. He'd been sitting at the cubicle of Lars, a senior developer, watching over his shoulder as Lars familiarized him with the application's UI.

"Garbage Collection." Using his mouse, Lars circled a button in the interface labeled just that. "We added this to solve a bug some users were experiencing. Now we just tell everyone that if they notice any weird behavior in the application, they should click this button."

Remco frowned. "What happens in the code when you click that?"

"It calls System.gc()."

But that wasn't even guaranteed to run! The Java virtual machine handled its own garbage collection. And in no universe did you want to put a worse-than-useless button in your UI and manipulate clients into thinking it did something. But Remco didn't feel confident enough to speak his mind. He kept silent and soldiered on.

When Remco was granted access to the codebase, it got worse. The whole thing was a pile of spaghetti full of similar design brillance that mostly worked well enough to satisfy clients, although there was a host of bugs in the bug tracker, some of which had been rotting there for over 7 years. Remco had been given the unenviable task of fixing the oldest ones.

Remco slogged through another few months. Eventually, he was tasked with implementing a new feature that was supposed to be similar to existing features already in the application. He checked these other features to see how they were coded, intending to follow the same pattern. As it turned out, each and every one of them had been implemented in a different, weird way. The wheel had been reinvented over and over, and none of the implementations looked like anything he ought to be imitating.

Flummoxed, Remco approached Lars' cubicle and explained his findings. "How should I proceed?" he finally asked.

Lars shrugged, and looked up from a running instance of the application. "I don't know." Lars turned back to his screen and pushed "Garbage Collect".

Fairly soon after that enlightening experience, Remco moved on. Conglomcorp is still going, though whether they've retained their garbage collection button is anyone's guess.

[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.