The Daily WTF: Curious Perversions in Information Technology

2023-02-20 Reply Admin

Presumably they didn't test against an argument of null, which is not too surprising (though deplorable). Null, for most programmers, means "something that will never happen" or at a greater remove "something that is going to throw an exception sooner or later, so why should I care?"

To quote Eric Lippert:

The whole thing is unnecessarily complicated. The language design could have been something like "If you implement a CompareTo method, you get all the operators for free."

Then again, if you're stupid enough to blatantly recurse on the first check of a static function, you deserve everything you get.

2023-02-20 Reply Admin

TRWTF: Why doesn't the language or IDE warn about this? It's not like it is hard to catch automatically.

Steve_The_Cynic · 2023-02-20 Reply Admin

TRWTF: Why doesn't the language or IDE warn about this? It's not like it is hard to catch automatically.

Probably because of a PHB saying, "Oh, we're getting pesky warnings. Turn off the warning for that, and turn off that GDMF build-breaking -Werror thing."

2023-02-20 Reply Admin

I am not sure what issue this was trying to resolve.

Even if it did not have that bug in, it would change != to NotNullAndNotEquals - a bit of an odd function, but if needed why not implement it as an extension with that name? On second thoughts, please don't.

2023-02-20 Reply Admin

Not sure that having either "catch" a recursion is a good idea. In either case you'd need a formal proof that the recursion is never going to terminate, which I think hits the Halting Problem, and even if not is going to be ... difficult.

In this specific case, I agree that the IDE could slap a wiggly line under it. But it's just one specific case amongst many. And my experience is that programmers tend to ignore wiggly lines.

tom103 · 2023-02-20 Reply Admin

TRWTF: Why doesn't the language or IDE warn about this? It's not like it is hard to catch automatically.

Some do. Rider does, as well as Visual Studio with ReSharper. It's not really a warning, just an icon in the left gutter that indicates there's a recursive call. A warning wouldn't make much sense, because recursion is sometimes intentional, and appropriate.

In this case, using a is null instead of a != null would fix this issue (the logic would still be broken, though, since it would always return false if a is null...)

The_MAZZTer · 2023-02-20 Reply Admin

Actually ANY invocation of this function will infinitely recurse.

The proper way to do it is use Object.ReferenceEquals or a.Equals (which they successfully do at one point) to avoid recursing by accident.

2023-02-20 Reply Admin

I stand corrected.

However, I would claim that is still not the proper way. (Difficult to say, because we're not told whether this is a standalone static method, or part of a class.)

Assuming it is part of a class (and it bloody well should be if you're going to override operators), then I'll update my Eric Lippert quote by suggesting that IEquatable is the solution. To whit:

public static bool operator ==(Box obj1, Box obj2)
    {
        if (ReferenceEquals(obj1, obj2)) 
            return true;
        if (ReferenceEquals(obj1, null)) 
            return false;
        if (ReferenceEquals(obj2, null))
            return false;
        return obj1.Equals(obj2);
    }
    public static bool operator !=(Box obj1, Box obj2) => !(obj1 == obj2);
    public bool Equals(Box other)
    {
        if (ReferenceEquals(other, null))
            return false;
        if (ReferenceEquals(this, other))
            return true;
        return //ToDo based on matrix props
    }
    public override bool Equals(object obj) => Equals(obj as Matrix);

    public override int GetHashCode()
    {
        unchecked
        {
            int hashCode = // ToDo based on matrix props
            return hashCode;
        }
    }

Melissa U · 2023-02-20 Reply Admin

It looks like they wanted to emulate database conditions where every comparison with NULL evaluates to false. Of course this means that (a != b) == !(a == b) isn't true any more. Since this method will fail immeditaly whatever vlaue b has (null or not null), I assume it was working initially and then incorrectly fixed. Maybe the check was if ((Object) a != null) ... and then someone thought "why did this dickhead cast to Object, anything as an Object? Let's remove that..."

2023-02-20 Reply Admin

Uhm, in your operator == example it would be preferable to test obj1 and obj2 against null first. Otherwise, if either is null...

2023-02-20 Reply Admin

That's why it's supposed to be a class method.

2023-02-20 Reply Admin

... though, then again, you could be right.

Serves me right for copypasta off StackOverflow. (A well-named site for the present WTF.) Rearrange the 1,2,3 ifs to be 3, 2, 1. Unless you really want to cope with the idea that null == null, which is technically not true inside a type system, since null has no type.

Steve_The_Cynic · 2023-02-20 Reply Admin

I am not sure what issue this was trying to resolve.

Probably a sorry tale of null pointer crashes when trying to do e.g.

  if ( myThing != yourThing )

when myThing is null.

WatersOfOblivion · 2023-02-20 Reply Admin

This case is simple, yes, but the general case is undecidable. And even trivial cases like this require something like Abstract Interpretation, which is an O(n^3) algorithm.

When working with compilers, it's shocking how often a tiny, tiny change can make something go from "super simple linear time algorithm" to "best case doubly exponential" or undecidable.

MaxiTB · 2023-02-20 Reply Admin

I am confused by this example.

First of the proper way to implement equality checks in C# is by implementing IEquatable and IEquatable<T>. Then you implement the operators. You never use Equal in an operator, because the whole point of operators is that they are static functions which are called 20x faster than virtual functions, which is a HUGE difference for such a simple operation which is often called so many times and that is the reason why they use at all and the difference why Java kills the planet (Rich Lander agrees :P).

Now implementing an operator <T,object> is... questionable. That makes no sense, because you only need fast optimized comparisons between Ts, so ehm yeah, that's the real WTF here which results in a ton of issue by not understanding the purpose of operators in the first place.

Besides the statement that object.operator== calls Equals is nonsense. There is no object.operator==, the compiler translate for all reference objects by default comparisons as pointer comparisons (simple IL ceq); obviously it doesn't call object.Equals, it doesn't even call RuntimeHelpers.Equals which is called by object.Equals.

So what would be the correct way to do it after all? First the operator makes no sense because you don't comes T to objects. Second you NEVER EVER call Equals in an operator. Third if this is new to you, you better stick to record classes/structs.

LorenPechtel · 2023-02-20 Reply Admin

Other than the recursion bug this seems to simply be a not equal comparison that special-cases nulls. Operator overloading is too much of a minefield for me but I've written comparison operators that will say null is not equal to null.

I strongly suspect this started out simply returning a.Equals(b) == false but then something happened where a null was compared to a different null and caused a problem--and we got this oops that was obviously never tested.

2023-02-20 Reply Admin

"Man, remember when Stackoverflow was good and useful?"

No

DJSpudplucker · 2023-02-22 Reply Admin

I'd prefer a bondage and discipline language where even trying: "public static bool operator !=(MatrixObjectKey a, object b) " throws a compile error... e.g. "what makes you think you can compare a MatrixObjectKey to something that might not be a MatrixObjectKey ??"

2023-02-26

Comment held for moderation.

Unequal Code

Leave a comment on “Unequal Code”