The lights are on here and the roof is intact and I'm grateful. Is anybody home? You decide.

Pharm fan Ian S. clucked "Perhaps they'll put those as dates on my headstone." If you're very lucky.

 

An anonymous reader blew the whistle on their child labor practices. "This institution exclusively uses drivers who aren't legally old enough to drive."

 

Greg A. grumbled "Glad that the important notice that there was no important notice was given such prominence in the official ACT web page." I have nothing more to add.

 

Regular reader Michael R. reported "I can confirm Hermes knows how to navigate the unknown."

 

Finally, faithful follower B.J.H. has been around here long enough to see this one over and over again. "For some reason people keep thinking zip codes are numbers just because they are composed of digits. When EPIC sent paper mail asking for money in December the envelope used a zip code of 1740 (and it was delivered). They solved leading zero issue by switching to base 36." Or it might just be base 26, no way to tell.

 

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

For testing networking systems, load simulators are useful: send a bunch of realistic looking traffic and see what happens as you increase the amount of sent traffic. These sorts of simulators often rely on being heavily multithreaded, since one computer can, if pushed, generate a lot of network traffic.

Thus, when Jonas inherited a heavily multithreaded system for simulating load, that wasn't a surprise. The surprise was that the developer responsible for it didn't really understand threading in Java. Probably in other languages too, but in this case, Java was what they were using.

public void startTraffic() { Configuration.instance.inititiateStatistics(); Statistics.instance.addStatisticListener(gui); if (t != null) { if (t.isAlive()) { t.destroy(); } } t = new Thread(this); t.start(); }

Look, this is not a good way to manage threads in Java. I don't know if I'd call it a WTF, but it's very much a "baby's first threading" approach. There are better abstractions around threads that would avoid the need to manage thread instances directly. I certainly don't love situations where a Runnable also manages its own thread instance.

This is almost certainly a race condition, but I don't know if this function is called from multiple threads (but I suspect it might be).

But what's more interesting is where this code gets called. You see, starting a thread could trigger an exception, so you need to handle that:

public void run() { while (true) { try { loaderMain.startTraffic(); break; } catch (Exception e) { System.out.println("Exception in main loader thread!"); e.printStackTrace(); } } }

Inside of an infinite loop, we try to start traffic. If we succeed, we break out of the loop. If we fail, well, we try and try again and again and again and again and again and again…

Jonas writes:

Since I'm the only one that dares to wade through the opaque mess of code that somehow, against all odds, manages to work most of the time, I get to fix it whenever it presents strange behavior.

I suspect it's going to present much more strange behavior in the future.

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

Sebastian is now maintaining a huge framework which, in his words, "could easily be reduced in size by 50%", especially because many of the methods in it are reinvented wheels that are already provided by .NET and specifically LINQ.

For example, if you want the first item in a collection, LINQ lets you call First() or FirstOrDefault() on any collection. The latter option makes handling empty collections easier. But someone decided to reinvent that wheel, and like so many reinvented wheels, it's worse.

public static LoggingRule FindFirst (this IEnumerable<LoggingRule> rules, Func<LoggingRule, bool> predicate) { foreach (LoggingRule rule in rules) { return rule; } return null; }

This function takes a list of logging rules and a function to filter the logging rules, starts a for loop to iterate over the list, and then simply returns the first element in the list, thus exiting the for loop. If the loop doesn't contain any elements, we return null.

From the signature, I'd expect this function to do filtering, but it clearly doesn't. It just returns the first element, period. And again, there's already a built-in function for that. I don't know why this is exists, but I especially dislike that it's so misleading.

There's only one positive to say about this: if you did want to reduce the size of the framework by 50%, it's easy to see where I'd start.

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

Today's code, at first, just looks like using literals instead of constants. Austin sends us this C#, from an older Windows Forms application:

if (e.KeyChar == (char)4) { // is it a ^D? e.Handled = true; DoStuff(); } else if (e.KeyChar == (char)7) { // is it a ^g? e.Handled = true; DoOtherStuff(); } else if (e.KeyChar == (char)Keys.Home) { e.Handled = true; SpecialGoToStart(); } else if (e.KeyChar == (char)Keys.End) { e.Handled = true; SpecialGoToEnd(); }

Austin discovered this code when looking for a bug where some keyboard shortcuts didn't work. He made some incorrect assumptions about the code- first, that they were checking for a KeyDown or KeyUp event, a pretty normal way to check for keyboard shortcuts. Under that assumption, a developer would compare the KeyEventArgs.KeyCode property against an enum- something like e.KeyCode == Keys.D && Keys.Control, for a CTRL+D. That's clearly not what's happening here.

No, here, they used the KeyPressEvent, which is meant to represent the act of typing. That gives you a KeyPressEventArgs with a KeyChar property- because again, it's meant to represent typing text not keyboard shortcuts. They used the wrong event type, as it won't tell them about modifier keys in use, or gracefully handle the home or end keys. KeyChar is the ASCII character code of the key press: which, in this case, CTRL+D is the "end of transmit" character in ASCII (4), and CTRL+G is the goddamn bell character (7). So those two branches work.

But home and end don't have ASCII code points. They're not characters that show up in text. They get key codes, which represent the physical key pressed, not the character of text. So (char)Keys.Home isn't really a meaningful operation. But the enum is still a numeric value, so you can still turn it into a character- it just turns into a character that emphatically isn't the home key. It's the "$". And Keys.End turns into a "#".

It wasn't very much work for Austin to move the event handler to the correct event type, and switch to using KeyCodes, which were both more correct and more readable.

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

"Boy, stringly typed data is hard to work with. I wish there were some easier way to work with it!"

This, presumably, is what Gary's predecessor said. Followed by, "Wait, I have an idea!"

public static Object createValue(String string) { Object value = parseBoolean(string); if (value != null) { return value; } value = parseInteger(string); if (value != null) { return value; } value = parseDouble(string); if (value != null) { return value; } return string; }

This takes a string, and then tries to parse it, first into a boolean, failing that into an integer, and failing that into a double. Otherwise, it returns the original string.

And it returns an object, which means you still get to guess what's in there even after this. You just get to guess what it returned, and hope you cast it to the correct type. Which means this almost certainly is called like this:

boolean myBoolField = (Boolean)createValue(someStringContainingABool);

Which makes the whole thing useless, which is fun.

Gary found this code in a "long since abandoned" project, and I can't imagine why it ended up getting abandoned.

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

It's just the same refrain, over and over.

"Time Travel! Again?" exclaimed David B. "I knew that Alaska is a good airline. Now I get to return at the start of a century. And not this century. The one before air flight began." To be fair, David, there never is just one first time for time travel. It's always again, isn't it?

 

"If it's been that long, I definitely need a holiday," headlined Craig N. "To be fair, all the destinations listed in the email were in ancient Greece, and not in countries that are younger than Jesus."

 

An anonymous reader reports "Upon being told my site was insecure because insufficient authorization, I clicked the provided link to read up on specifics of the problem and suggestions for how to resolve it. To my surprise, Edge blocked me, but I continued on bravely only to find...this."

 

Footie fan Morgan has torn his hair out over this. "For the life of me I can't work out how this table is calculated. It's not just their league either. Others have the same weird positioning of teams based on their points. It must be pointed out that this is the official TheFA website as well not just some hobbyist site." It's too late for me, but I'm frankly baffled as well.

 

Most Excellent Stephen is stoked to send us off with this. "Each year we have to renew the registration on our vehicles. It is not something we look forward to no matter which state you live in. A few years ago Texas introduced an online portal for this which was an improvement, if you didn't wait until the last minute of course. Recently they added a feature to the portal to track the progress of your renewal and see when they mail the sticker to you. I was pleasantly surprised to see the status page."

 

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

DZ's tech lead is a doctor of computer science, and that doctor loves to write code. But you already know that "PhD" stands for "Piled high and deep", and that's true of the tech lead's clue.

For example, in C#:

private List<Foo> ExtractListForId(string id) { List<Foo> list = new List<Foo>(); lock (this) { var items = _foos.Where(f => f.Id == id).ToList(); foreach (var item in items) { list.Add(item); } } return list; }

The purpose of this function is to find all the elements in a list where they have a matching ID. That's accomplished in one line: _foo.Where(f => f.Id == id). For some reason, the function goes through the extra step of iterating across the returned list and constructing a new one. There's no real good reason for this, though it does force LINQ to be eager- by default, the Where expression won't be evaluated until you check the results.

The lock is in there for thread safety, which hey- the enumerator returned by Where is not threadsafe, so that's not a useless thing to do there. But it's that lock which hints at the deeper WTF here: our PhD-having-tech-lead knows that adding threads ensures you're using more of the CPU, and they've thrown threads all over the place without any real sense to it. There's no clear data ownership of any given thread, which means everything is locked to hell and back, the whole thing frequently deadlocks, and it's impossible to debug.

It's taken days for DZ to get this much of a picture of what's going on in the code, and further untangling of this multithreaded pile of spaghetti is going to take many, many more days- and much, much more of DZ's sanity.

[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 takes a lot of time and effort to build a code base that exceeds 100kloc. Rome wasn't built in a day; it just burned down in one.

Liza was working in a Python shop. They had a mildly successful product that ran on Linux. The sales team wanted better sales software to help them out, and instead of buying something off the shelf, they hired a C# developer to make something entirely custom.

Within a few months, that developer had produced a codebase of 320kloc I say "produced" and not "wrote" because who knows how much of it was copy/pasted, stolen from Stack Overflow, or otherwise not the developer's own work.

You have to wonder, how do you get such a large codebase so quickly?

private String getDatum() { DateTime datum = new DateTime(); datum = DateTime.Now; return datum.ToShortDateString(); } public int getTag() { int tag; DateTime datum = new DateTime(); datum = DateTime.Today; tag = datum.Day; return tag; } private int getMonat() { int monat; DateTime datum = new DateTime(); datum = DateTime.Today; monat = datum.Month; return monat; } private int getJahr() { int monat; DateTime datum = new DateTime(); datum = DateTime.Today; monat = datum.Year; return monat; } private int getStunde() { int monat; DateTime datum = new DateTime(); datum = DateTime.Now; monat = datum.Hour; return monat; } private int getMinute() { int monat; DateTime datum = new DateTime(); datum = DateTime.Now; monat = datum.Minute; return monat; }

Instead of our traditional "bad date handling code" which eschews the built-in libraries, this just wraps the built in libraries with a less useful set of wrappers. Each of these could be replaced with some version of DateTime.Now.Minute.

You'll notice that most of the methods are private, but one is public. That seems strange, doesn't it? Well this set of methods was pulled from one random class which implements them in the codebase, but many classes have these methods copy/pasted in. At some point, the developer realized that duplicating that much code was a bad idea, and started marking them as public, so that you could just call them as needed. Note, said developer never learned to use the keyword static, so you end up calling the method on whatever random instance of whatever random class you happen to have handy. The idea of putting it into a common base class, or dedicated date-time utility class never occurred to the developer, but I guess that's because they were already part of a dedicated date-time utility class.

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

When Steve's employer went hunting for a new customer relationship management system (CRM), they had some requirements. A lot of them were around the kind of vendor support they'd get. Their sales team weren't the most technical people, and the company wanted to push as much routine support off to the vendor as possible.

But they also needed a system that was extensible. Steve's company had many custom workflows they wanted to be able to execute, and automated marketing messages they wanted to construct, and so wanted a CRM that had an easy to use API.

"No worries," the vendor sales rep said, "we've had a RESTful API in our system for years. It's well tested and reliable. It's JSON based."

The purchasing department ground their way through the purchase order and eventually they started migrating to the new CRM system. And it fell to Steve to start learning the JSON-based, RESTful API.

"JSON"-based was a more accurate description.

For example, an API endpoint might have a schema like:

DeliveryId: int // the ID of the created delivery Errors: xml // Collection of errors encountered

This example schema is representative. Many "JSON" documents contained strings of XML inside of them.

Often, this is done when an existing XML-based API is "modernized", but in this case, the root cause is a little dumber than that. The system uses SQL Server as its back end, and XML is one of the native types. They just have a stored procedure build an XML object and then return it as an output parameter.

You'll be surprised to learn that the vendor's support team had a similar level of care: they officially did what you asked, but sometimes it felt like malicious compliance.

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

A common class of bad code is the code which mixes server side code with client side code. This kind of thing:

<script> <?php if (someVal) { ?> var foo = <? echo someOtherVal ?>; <?php } else { ?> var foo = 5; <?php } ?> </script>

We've seen it, we hate it, and is there really anything new to say about it?

Well, today's anonymous submitter found an "interesting" take on the pattern.

<script> if(linkfromwhere_srfid=='vff') { <?php $vff = 1; ?> } </script>

Here, they have a client-side conditional, and based on that conditional, they attempt to set a variable on the server side. This does not work. This cannot work: the PHP code executes on the server, the client code executes on the client, and you need to be a lot more thoughtful about how they interact than this.

And yet, the developer responsible has done this all over the code base, pushed the non-working code out to production, and when it doesn't work, just adds bug tickets to the backlog to eventually figure out why- tickets that never get picked up, because there's always something with a higher priority out there.

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

Faithful Peter G. took a trip. "So I wanted to top up my bus ticket online. After asking for credit card details, PINs, passwords, a blood sample, and the airspeed velocity of an unladen European swallow, they also sent a notification to my phone which I had to authorise with a fingerprint, and then verify that all the details were correct (because you can never be too careful when paying for a bus ticket). So yes, it's me, but the details definitely are not correct." Which part is wrong, the currency? Any idea what the exchange rate is between NZD and the euro right now?

 

An anonymous member kvetched "Apparently, I'm a genius, but The New York Times' Spelling Bee is definitely not."

 

Mickey D. Had an ad pop up for a new NAS to market.
Specs: Check
Storage: Check
Superior technical support: "

 

Michael R. doesn't believe everything he sees on TV, thankfully, because "No wonder the stock market is in turmoil when prices fall by 500% like in the latest Amazon movie G20."

 

Finally, new friend Sandro shared his tale of woe. "Romance was hard enough I was young, and I see not much has changed now!"

 

[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 was reviewing some C++ code, and saw this perfectly reasonable looking pattern.

class SomeClass { public: void setField(int val); int getField(); }

Now, we can talk about how overuse of getters and setters is itself an antipattern (especially if they're trivial- you've just made a public variable with extra steps), but it's not wrong and there are certainly good reasons to be cautious with encapsulation. That said, because this is C++, that getField should really be declared int getField() const- appropriate for any method which doesn't cause a mutation to a class instance.

Or should it? Let's look at the implementation.

void SomeClass::setField(int val) { setGetField(true, val); } void SomeClass::getField() { return setGetField(false); }

Wait, what? Why are we passing a boolean to a method called setGet. Why is there a method called setGet? They didn't go and make a method that both sets and gets, and decide which they're doing based on a boolean flag, did they?

int SomeClass::setGetField(bool set, int val) { static int s_val = 0; if (set) { s_val = val; } return s_val; }

Oh, good, they didn't just make a function that maybe sets or gets based on a boolean flag. They also made the state within that function a static field. And yes, function level statics are not scoped to an instance, so this is shared across all instances of the class. So it's not encapsulated at all, and we've blundered back into Singletons again, somehow.

Our anonymous submitter had two reactions. Upon seeing this the first time, they wondered: "WTF? This must be some kind of joke. I'm being pranked."

But then they saw the pattern again. And again. After seeing it fifty times, they wondered: "WTF? Who hired these developers? And can that hiring manager be fired? Out of a cannon? Into the sun?"

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

Now, I would argue that the event-driven lifecycle of ASP .Net WebForms is a bad way to design web applications. And it's telling that the model is basically dead; it seems my take is at best lukewarm, if not downright cold.

Pete inherited code from Bob, and Bob wrote an ASP .Net WebForm many many ages ago, and it's still the company's main application. Bob may not be with the company, but his presence lingers, both in the code he wrote and the fact that he commented frequently with // bob was here

Bob liked to reinvent wheels. Maybe that's why most methods he wrote were at least 500 lines long. He wrote his own localization engine, which doesn't work terribly well. What code he did write, he copy/pasted multiple times.

He was fond of this pattern:

if (SomeMethodReturningBoolean()) { return true; } else { return false; }

Now, in a Web Form, you usually attach your events to parts of the page lifecycle by convention. Name a method Page_Load? It gets called when the load event fires. Page_PreRender? Yep- when the pre-render event fires. SomeField_MouseClick? You get it.

Bob didn't, or Bob didn't like coding by naming convention. Which, I'll be frank, I also don't like coding by naming convention, but it was the paradigm Web Forms favored, it's what the documentation assumed, it's what every other developer was going to expect to see.

Still, Bob had his own Bob way of doing it.

In every page he'd write code like this:

this.PreRender += this.RequestPagePreRender

That line manually registers an event handler, which invokes the method RequestPagePreRender. And while I might object to wiring up events by convention- this is still just a convention. It's not done with any thought at all- every page has this line, even if the RequestPagePreRender method is empty.

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

Mark was debugging some database querying code, and got a bit confused about what it was actually doing. Specifically, it generated a query block like this:

$statement="declare @status int declare @msg varchar(30) exec @status=sp_doSomething 'arg1', ... select @msg=convert(varchar(10),@status) print @msg "; $result = sybase_query ($statement, $this->connection);

Run a stored procedure, capture its return value in a variable, stringify that variable and print it. The select/print must be for debugging, right? Leftover debugging code. Why else would you do something like that?

if (sybase_get_last_message()!=='0') { ... }

Oh no. sybase_get_last_message gets the last string printed out by a print statement. This is a pretty bonkers way to get the results of a function or procedure call back, especially when if there are any results (like a return value), they'll be in the $result return value.

Now that said, reading through those functions, it's a little unclear if you can actually get the return value of a stored procedure this way. Without testing it myself (and no, I'm not doing that), we're in a world where this might actually be the best way to do this.

So I'm not 100% sure where the WTF lies. In the developer? In the API designers? Sybase being TRWTF is always a pretty reliable bet. I suppose there's a reason why all those functions are listed as "REMOVED IN PHP 7.0.0", which was was rolled out through 2015. So at least those functions have been dead for a decade.

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

We talked about singletons a bit last week. That reminded John of a story from the long ago dark ages where we didn't have always accessible mobile Internet access.

At the time, John worked for a bank. The bank, as all banks do, wanted to sell mortgages. This often meant sending an agent out to meet with customers face to face, and those agents needed to show the customer what their future would look like with that mortgage- payment calculations, and pretty little graphs about equity and interest.

Today, this would be a simple website, but again, reliable Internet access wasn't a thing. So they built a client side application. They tested the heck out of it, and it worked well. Sales agents were happy. Customers were happy. The bank itself was happy.

Time passed, as it has a way of doing, and the agents started clamoring for a mobile web version, that they could use on their phones. Now, the first thought was, "Wire it up to the backend!" but the backend they had was a mainframe, and there was a dearth of mainframe developers. And while the mainframe was the source of truth, and the one place where mortgages actually lived, building a mortgage calculator that could do pretty visualizations was far easier- and they already had one.

The client app was in .NET, and it was easy enough to wrap the mortgage calculation objects up in a web service. A quick round of testing of the service proved that it worked just as well as the old client app, and everyone was happy - for awhile.

Sometimes, agents would run a calculation and get absolute absurd results. Developers, putting in exactly the same values into their test environment wouldn't see the bad output. Testing the errors in production didn't help either- it usually worked just fine. There was a Heisenbug, but how could a simple math calculation that had already been tested and used for years have a Heisenbug?

Well, the calculation ran by simulation- it simply iteratively applied payments and interest to generate the entire history of the loan. And as it turns out, because the client application which started this whole thing only ever needed one instance of the calculator, someone had made it a singleton. And in their web environment, this singleton wasn't scoped to a single request, it was a true global object, which meant when simultaneous requests were getting processed, they'd step on each other and throw off the iteration. And testing didn't find it right away, because none of their tests were simulating the effect of multiple simultaneous users.

The fix was simple- stop being a singleton, and ensure every request got its own instance. But it's also a good example of misapplication of patterns- there was no need in the client app to enforce uniqueness via the singleton pattern. A calculator that holds state probably shouldn't be a singleton in the first place.

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

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.