Had this bug come back to me and dug further

that_really_other_gu

This is from something that's getting gutted and re-written... the old system was a huge pile of fail... we called it through a web service and part of that involved passing a giant xml string....

and part of that that affected the way things looked was this little gem....

(this is c# and nullableBit is what equates to a bool?)....

nullableBit.GetValueOrDefault(false) ? "false" : "true"

I want to hold the persons head that did this and make them look at it and "Look, look what you've done".. and hit them with a rolled up something... newspaper... magazine.... steel....

Steeldragon

The thing you rae looking for is a ClueByFour. *Hands trog one as it is made of nice oak wood.*

I think I need more context on this one. I suppose you weren't expected to invert the semantics of nullableBit, is that it?

Ragnax

@Zecc said:

I think I need more context on this one. I suppose you weren't expected to invert the semantics of nullableBit, is that it?

That would be the only WTF I can spot as well. GetValueOrDefault is likely an extension method on Nullable<T> or similar, which means its output is constrained to be the same as the generic type parameter, i.e. , boolean. You'd need to convert that to a string. The first thought will be to just invoke ToString, but when invoking a boolean's ToString it will output a capitalized "True" or "False", not a lower case "true" or "false".

As string literals are automatically interned by the C# compiler, just using a ternary operator on the boolean and outputting either the string literal "true" or "false" is likely more efficient than invoking ToString followed by ToLowerInvariant(), which has to walk the current string to perform case conversion and put the result into a second, possibly non-interned, string.

mt11

Pretty sure that the wtf is that it return "false" if getValueOrDefault returns true, and "true" if getValueOrDefault return false.

that_really_other_gu

I think I anonimized it too much... the nullableBit name was something more like a flag to know if we should reset a counter...

so... more like....

shouldReset.getValueOrDefault(false) ? "false" : "true"

but... that value comes from a database and it's NOT hard to fix the values... it's like whoever set the values in this table didn't understand why they were setting them... and there's a ton of null values in that table.. (which any sane person would assume would mean false...)

It's like they decided.. hmm I can fix this, or I can do this hacky thing that no one will ever find and save myself an hour of fixing the data....

that_really_other_gu

And that thing is a Nullable<T> but it's getting appended to a manually generated string of xml... even if they'd done this

!shouldReset.getValueOrDefault(false) ? "true" : "false"

that would at least have been a better warning for weird things ahead :)....

Ben L.

@that really other guy said:

And that thing is a Nullable<T> but it's getting appended to a manually generated string of xml... even if they'd done this
!shouldReset.getValueOrDefault(false) ? "true" : "false"
that would at least have been a better warning for weird things ahead :)....

Is this going to become one of those "make the code return false in a really hard-to-read way" threads?

I love those threads.

morbiuswilters

@Ben L. said:

Is this going to become one of those "make the code return false in a really hard-to-read way" threads?

<html><body style='color:#55cc66; background:#001800; '>public class Whatever
{
    public static void main(String[] args)
    {
        Thing t = new Thing(args[0].equals("true"));

        System.out.println("t.isTrue(): " + (t.isTrue() ? "true" : "false"));
    }
}

class Thing
{
    private boolean isTrue;

    Thing(boolean t)
    {
        isTrue = t;
    }
    boolean isTrue()
    {
        try {
            if (isTrue) {
                return isTrue();
            } else {
                String s = "f";

                while (true) {
                    s += s;
                }
            }
        } catch (StackOverflowError e) {
            return true;
        } catch (OutOfMemoryError e) {
            return false;
        }
    }
}

Jaime

@that really other guy said:

It's like they decided.. hmm I can fix this, or I can do this hacky thing that no one will ever find and save myself an hour of fixing the data....

Fixing the data is fine if there is a problem. But, no amount of fixing the data will get around the fact that nulls are allowed and the code has to do something when the value is null. The only two appropriate courses of action are to fix the schema or to properly handle nulls. Fixing the data is a work-around, not a fix.

@morbiuswilters said:

if (isTrue) {

return isTrue();

}

Not enough reflection.

Medinoc

This... This is beautiful...

mikeTheLiar

@morbiuswilters said:

public class Whatever

The sad thing is that despite how pants-shittingly bad that is, it's at least cleverly bad. I don't think anyone I work with would be able to understand that, let alone think it up on their own.

MiffTheFox

Bonus points for the ugly green-on-more-green color scheme.

dkf

@Zecc said:

Not enough reflection.

Not enough JEE readiness; it needs a TruthinessFactoryProviderStrategy which can be then looked up in JNDI (so that it can be configured during deployment to be an instance of FoxNews).