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

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?"

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

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. You just can't use it.

And you can't use it because of the real "magic" here: DoSomething, which checks if the currently active instance is also the originally 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.