How to fuck up op_Equality


  • Considered Harmful

    We use Very Expensive Enterprisey Web CMS. It's full of stupid, but I recently found it would crash whenever I tried to check a ShortID (it's a Guid with the brackets and hyphens stripped off) for null. Apparently this is how they implemented it:

    public static bool operator ==( ShortID id1, ShortID id2 ) {
    	return id1.Guid == id2.Guid;
    }
    

    public static bool operator !=( ShortID id1, ShortID id2 ) {
    return id1.Guid != id2.Guid;
    }



    It accesses a property of the compared object without checking it for null, so if you do shortId == null, you get a NullReferenceException. Wee.

    I'm trying to figure out a workaround that doesn't involve reimplementing ShortID. Not that it would be difficult, but using the API insulates us from inevitable changes in underlying data structures.



    Update: just remembered Object.ReferenceEquals()


  • Considered Harmful

    Bonus WTF: on many classes they've overloaded op_Equality. Do you know why you're not supposed to overload op_Equality?

    Hint:
    CS0121: The call is ambiguous between the following methods or properties:
    'WTFCMS.Data.ID.operator ==(WTFCMS.Data.ID, WTFCMS.Data.ID)' and
    'WTFCMS.Data.ID.operator ==(WTFCMS.Data.ID, WTFCMS.Data.ShortID)'



  • @joe.edwards said:

    It accesses a property of the compared object without checking it for null, so if you do shortId == null, you get a NullReferenceException.

    Then don't do that.



  • @Ronald said:

    @joe.edwards said:

     

    It accesses a property of the compared object without checking it for null, so if you do shortId == null, you get a NullReferenceException.

    Then don't do that.

     



  • @joe.edwards said:

    Apparently this is how they implemented it:

    public static bool operator ==( ShortID id1, ShortID id2 ) {
    	return id1.Guid == id2.Guid;
    }
    

    public static bool operator !=( ShortID id1, ShortID id2 ) {
    return id1.Guid != id2.Guid;
    }



    It accesses a property of the compared object without checking it for null, so if you do shortId == null, you get a NullReferenceException. Wee.

    apparently you have access to the source, so what is your reason not to edit it there? adding null check to the operators solves the issue and doesn't change ShortId's data structure, does it?



  • @SEMI-HYBRID code said:

    apparently you have access to the source, so what is your reason not to edit it there? adding null check to the operators solves the issue and doesn't change ShortId's data structure, does it?

    Last time I put a null check in some common code, a null pointer exception was no longer swallowed, causing a broken validation function to go straight into production to fuck things up. Yeah, TRWTF is no unit tests or code coverage metrics.


  • Considered Harmful

    @SEMI-HYBRID code said:

    @joe.edwards said:

    Apparently this is how they implemented it:

    public static bool operator ==( ShortID id1, ShortID id2 ) {
    	return id1.Guid == id2.Guid;
    }
    

    public static bool operator !=( ShortID id1, ShortID id2 ) {
    return id1.Guid != id2.Guid;
    }



    It accesses a property of the compared object without checking it for null, so if you do shortId == null, you get a NullReferenceException. Wee.

    apparently you have access to the source, so what is your reason not to edit it there? adding null check to the operators solves the issue and doesn't change ShortId's data structure, does it?


    I do not have access to the source, nor would changing upstream code be acceptable if I did. This is disassembled from the CIL.



  • Or maybe he just divined the source code. How many different ways could you implement the == operator so that you'd get a NullReferenceException when a ShortID is null? [color=inherit;display:none](Don't answer that.)[/color]



  • The problem here seems to me to be one of inconsistent semantics expected by the code using and the code implementing the equality operator.

    If you write shortId == null, you are expecting the equality operator to be comparing references.

    If you write return id1.Guid == id2.Guid, you are expecting the equality operator to be comparing values structurally.




  • Reminds me of a bug I ran into while using a third-party library at my last job. One of the library methods was making a web call using HTTP GET instead of POST with a parameter that was too long for GET, causing errors. Naturally this was for a feature the bosses deemed "mission-critical." We called the vendor of the library and they said they were aware of the issue and it was slated to be fixed with their next release in 18 months. So I set about fixing it myself. I decompiled their library (it was Silverlight so Reflector had no problem with it) and fixed the issue but was then unable to recompile because it was highly obfuscated and I couldn't get my C# compiler to actually accept their obfuscated code. So I spent a while fixing those issues and switching class and method names to something the compiler would accept, and it eventually compiled. Then I had problems getting it to work with the other libraries from this vendor (there were 3 or 4 DLLs and I only worked on one). They were all signed with code-signing certificates, except the one I decompiled, so they quit playing nice. I gave up. The bosses weren't happy but there was nothing I could do about it.

    Fixing other people's code is sometimes impossible.



  • @anotherusername said:

    How many different ways could you implement the == operator
     

    int Ways = 0;

    while (Rose.smell == FlowerX.smell) {
        Ways++;
    }



  • @mott555 said:

    I
     

    I appreciated your icon, and then I looked more closely, and then I appreciated it even more.


  • Considered Harmful

    @dhromed said:

    @mott555 said:

    I
     

    I appreciated your icon, and then I looked more closely, and then I appreciated it even more.


    Did you appreciate it with your pants off?


  • Considered Harmful

    @mott555 said:

    Reminds me of a bug I ran into while using a third-party library at my last job. One of the library methods was making a web call using HTTP GET instead of POST with a parameter that was too long for GET, causing errors. Naturally this was for a feature the bosses deemed "mission-critical." We called the vendor of the library and they said they were aware of the issue and it was slated to be fixed with their next release in 18 months. So I set about fixing it myself. I decompiled their library (it was Silverlight so Reflector had no problem with it) and fixed the issue but was then unable to recompile because it was highly obfuscated and I couldn't get my C# compiler to actually accept their obfuscated code. So I spent a while fixing those issues and switching class and method names to something the compiler would accept, and it eventually compiled. Then I had problems getting it to work with the other libraries from this vendor (there were 3 or 4 DLLs and I only worked on one). They were all signed with code-signing certificates, except the one I decompiled, so they quit playing nice. I gave up. The bosses weren't happy but there was nothing I could do about it.

    Fixing other people's code is sometimes impossible.

    It would be a total WTF, but you could probably use code emission to patch code at runtime, so you can avoid tampering with signed code directly.

    I'm not personally fond of fixing WTFs with bigger WTFs. Even if your patched code worked, what do you do when there's a newer version?



  • @joe.edwards said:

    It would be a total WTF, but you could probably use code emission to patch code at runtime, so you can avoid tampering with signed code directly.

    I'm not personally fond of fixing WTFs with bigger WTFs. Even if your patched code worked, what do you do when there's a newer version?

    I tried Reflection and CIL injection at runtime, but Silverlight didn't really support that and I had all kinds of odd exceptions.

    All of this happened because the higher-ups didn't understand software development. There was a lot of WTF-y things I wrote just to sidestep issues in the vendor's API. Most of them involved re-writing one of their features from scratch because their implementation supported A, B, C, D, but not feature E which is what the boss really wanted. Of course all their classes were sealed, so I'd end up writing my own implementation of A, B, C, and D as well because there was no way to just write E on its own. We can't possibly wait until the vendor provides it because we need it Right Freaking Now™.

    It got to the point that I recommended against using the vendor's API and just writing our own. I was told no because it would take too long. Then I'd get hit with orders requiring me to create my own API anyway, since vendor's API didn't quite support it.

     



  • @joe.edwards said:

    We use Very Expensive Enterprisey Web CMS. It's full of stupid, but I recently found it would crash whenever I tried to check a ShortID (it's a Guid with the brackets and hyphens stripped off) for null. Apparently this is how they implemented it:

    public static bool operator ==( ShortID id1, ShortID id2 ) {
    	return id1.Guid == id2.Guid;
    }
    

    public static bool operator !=( ShortID id1, ShortID id2 ) {
    return id1.Guid != id2.Guid;
    }



    It accesses a property of the compared object without checking it for null, so if you do shortId == null, you get a NullReferenceException. Wee.

    I'm trying to figure out a workaround that doesn't involve reimplementing ShortID. Not that it would be difficult, but using the API insulates us from inevitable changes in underlying data structures.



    Update: just remembered Object.ReferenceEquals()

     

    How the unholy fuck did you format your code like that? Manually in the html editor? In some other code-formatting tool that spits out html? Tell me your secrets, wise one.

     


  • Considered Harmful

    @Mo6eB said:

    How the unholy fuck did you format your code like that? Manually in the html editor? In some other code-formatting tool that spits out html? Tell me your secrets, wise one.

    It's an ancient Cantonese secret.



  • @joe.edwards said:

    @Mo6eB said:
    How the unholy fuck did you format your code like that? Manually in the html editor? In some other code-formatting tool that spits out html? Tell me your secrets, wise one.

    It's an ancient Cantonese secret.

    I'm prefer the ancient Russian secret.


  • Considered Harmful

    @mikeTheLiar said:

    @joe.edwards said:
    @Mo6eB said:
    How the unholy fuck did you format your code like that? Manually in the html editor? In some other code-formatting tool that spits out html? Tell me your secrets, wise one.

    It's an ancient Cantonese secret.

    I'm prefer the ancient Russian secret.

    In ancient Soviet Russia, joke gets old.


  • Considered Harmful

    @DaveK said:

    The problem here seems to me to be one of inconsistent semantics expected by the code using and the code implementing the equality operator.

    If you write shortId == null, you are expecting the equality operator to be comparing references.

    If you write return id1.Guid == id2.Guid, you are expecting the equality operator to be comparing values structurally.


    Regardless of semantics, I always consider a method that throws a NRE to be broken. If you don't accept null as an argument, make that part of the contract and throw ArgumentNullException. NullReferenceException is a logic error. One I was trying to avoid, by checking for null!



  • http://www.cs.uwm.edu/~cs654/handouts/cool-manual.pdf

    Relevant snippet:

    The comparison == is syntactic sugar: e1 == e2 is parsed into an internal form equivalent to e1.equals(e2) and thus will crash at runtime if e1 is null.

  • Considered Harmful

    @Ben L. said:

    http://www.cs.uwm.edu/~cs654/handouts/cool-manual.pdf

    Relevant snippet:

    The comparison == is syntactic sugar: e1 == e2 is parsed into an internal form equivalent to e1.equals(e2) and thus will crash at runtime if e1 is null.

    This crashes if e2 is null. (Or e1)

    if( notNullShortId != null ) { // NullReferenceException
        // use notNullShortId
    }
    


  • @joe.edwards said:

    @Ben L. said:
    http://www.cs.uwm.edu/~cs654/handouts/cool-manual.pdf

    Relevant snippet:

    The comparison == is syntactic sugar: e1 == e2 is parsed into an internal form equivalent to e1.equals(e2) and thus will crash at runtime if e1 is null.

    This crashes if e2 is null. (Or e1)
    Well fuck.

    Let's design a new processor architecture based on x86 that just crashes instead of executing instructions. That'll save a lot of time.



  • @Ben L. said:

    http://www.cs.uwm.edu/~cs654/handouts/cool-manual.pdf

    Relevant snippet:

    The comparison == is syntactic sugar: e1 == e2 is parsed into an internal form equivalent to e1.equals(e2) and thus will crash at runtime if e1 is null.

    Wow.
    Question; does the forum rule regarding the avoidance of work done in an educational setting also extend to the course material itself?
    If not; I wonder what other WTFs are hidden in that thing...



  • @Ragnax said:

    @Ben L. said:
    http://www.cs.uwm.edu/~cs654/handouts/cool-manual.pdf

    Relevant snippet:

    The comparison == is syntactic sugar: e1 == e2 is parsed into an internal form equivalent to e1.equals(e2) and thus will crash at runtime if e1 is null.

    Wow.
    Question; does the forum rule regarding the avoidance of work done in an educational setting also extend to the course material itself?
    If not; I wonder what other WTFs are hidden in that thing...

    There are no boolean operators and no statements, so to write x && y && z you need to write: if (x) if (y) z else false else false

    There's also no "greater than" operator.

    Oh, and it only runs on MIPS.



  • @Ben L. said:

    @Ragnax said:
    @Ben L. said:
    http://www.cs.uwm.edu/~cs654/handouts/cool-manual.pdf [uwm.edu]

    Relevant snippet:

    The comparison == is syntactic sugar: e1 == e2 is parsed into an internal form equivalent to e1.equals(e2) and thus will crash at runtime if e1 is null.

    Wow.
    Question; does the forum rule regarding the avoidance of work done in an educational setting also extend to the course material itself?
    If not; I wonder what other WTFs are hidden in that thing...

    There are no boolean operators and no statements, so to write x && y && z you need to write: if (x) if (y) z else false else false

    There's also no "greater than" operator.

    Oh, and it only runs on MIPS.

    Well, that much at least is a relief. It's guaranteed to stay far, far away from me.


  • @anotherusername said:

    @Ben L. said:
    @Ragnax said:
    @Ben L. said:
    http://www.cs.uwm.edu/~cs654/handouts/cool-manual.pdf [uwm.edu]

    Relevant snippet:

    The comparison == is syntactic sugar: e1 == e2 is parsed into an internal form equivalent to e1.equals(e2) and thus will crash at runtime if e1 is null.

    Wow.
    Question; does the forum rule regarding the avoidance of work done in an educational setting also extend to the course material itself?
    If not; I wonder what other WTFs are hidden in that thing...

    There are no boolean operators and no statements, so to write x && y && z you need to write: if (x) if (y) z else false else false

    There's also no "greater than" operator.

    Oh, and it only runs on MIPS.

    Well, that much at least is a relief. It's guaranteed to stay far, far away from me.
    Extended Cool runs on the JVM.


  • Side note: that's bizarre, the Quote button somehow included the text that my Greasemonkey script added after that link.


  • Discourse touched me in a no-no place

    @Ragnax said:

    does the forum rule regarding the avoidance of work done in an educational setting also extend to the course material itself
    It's written by someone who really ought to know better and who ought to check their facts, etc. It's totally fair game.



  • @joe.edwards said:

    @dhromed said:
    @mott555 said:
    I
    I appreciated your icon, and then I looked more closely, and then I appreciated it even more.
    Did you appreciate it with your pants off?
    It's TDWTF123 and possibly Boomzilla who get that excited over Excel. But I suspect Boomzilla only does it for the lulz.


  • ♿ (Parody)

    @mott555 said:

    @joe.edwards said:
    @dhromed said:
    @mott555 said:

    I appreciated your icon, and then I looked more closely, and then I appreciated it even more.

    Did you appreciate it with your pants off?

    It's TDWTF123 and possibly Boomzilla who get that excited over Excel. But I suspect Boomzilla only does it for the lulz.

    But everyone should get as excited about it, you fucking ivory tower excel denier. Go ahead and waste time and money on your bespoke code, while the rest of us bring it turbo charged with Excel.



  • The joke's on you! That's not Excel in my avatar, it's OpenOffice's Calc! I hope that makes you feel like the guy who had the most satisfying night ever with an unimaginably beautiful woman, only to find out she used to be a man.



  • @mott555 said:

    OpenOffice's Calc
    What are you, a bassoon player?


  • ♿ (Parody)

    @mott555 said:

    The joke's on you! That's not Excel in my avatar, it's OpenOffice's Calc! I hope that makes you feel like the guy who had the most satisfying night ever with an unimaginably beautiful woman, only to find out she used to be a man.

    Wait...how did you know what I did last night? Are you sure about this?



  • Calc's just that much better! Excel might solve all your computing problems, but switch to Calc and you can be psychic too!



  • I switched to Calc once and it psychically knew that I wanted all of my "Scatter" plots to connect the points with curves instead of straight line segments.

    Unfortunately for Calc, me and Excel just couldn't live up to its expectations of us, so it's probably still doing its own thing somewhere while I'm content to putz around in Excel.


  • Considered Harmful

    @boomzilla said:

    @mott555 said:
    The joke's on you! That's not Excel in my avatar, it's OpenOffice's Calc! I hope that makes you feel like the guy who had the most satisfying night ever with an unimaginably beautiful woman, only to find out she used to be a man.

    Wait...how did you know what I did last night? Are you sure about this?


    I thought we had something special!



  • @Ragnax said:

    Wow.
    Question; does the forum rule regarding the avoidance of work done in an educational setting also extend to the course material itself?
    If not; I wonder what other WTFs are hidden in that thing...
    The forum rule only applies to student work product; instructors, as they most definitely should know better, are open for mocking.



  • @joe.edwards said:

    I thought we had something special!

    TRWTF is not configuring the content expiration on your big sig banners so they don't redownload every time a page is refreshed.

    Maybe we should all chip in and fund a community CloudFront account for the signatures.*

    * I'm just kidding, what a bunch of commies


  • Considered Harmful

    @Ronald said:

    @joe.edwards said:
    I thought we had something special!

    TRWTF is not configuring the content expiration on your big sig banners so they don't redownload every time a page is refreshed.

    Maybe we should all chip in and fund a community CloudFront account for the signatures.*

    * I'm just kidding, what a bunch of commies

    I explicitly configured them to expire immediately so that it would show a different message every time.


  •  I once found a case of (anonymised from memory, not cut and paste from source...):

      operator == (point a, point b)

    {

          return (a.x == b.x && a.y == b.y);

    }

      operator != (point a, point b)

    {

          return (a.x != b.x && a.y != b.y);

     }

    Note the second && which should be a ||

     The inequality operator is kinda important for .net collection classes like dictionary keys. This caused duplicate keys to end up in a dictionary under stress testing...  this was in a published SDK from 'a large company in redmond'.


  • Considered Harmful

    @bgodot said:

     I once found a case of (anonymised from memory, not cut and paste from source...):

      operator == (point a, point b)

    {

          return (a.x == b.x && a.y == b.y);

    }

      operator != (point a, point b)

    {

          return (a.x != b.x && a.y != b.y);

     }

    Note the second && which should be a ||

     The inequality operator is kinda important for .net collection classes like dictionary keys. This caused duplicate keys to end up in a dictionary under stress testing...  this was in a published SDK from 'a large company in redmond'.


    Is there any time you'd want x != y to be different from !( x == y )? I don't know why you even need to define != separately from ==.



  • @joe.edwards said:

    @Ronald said:
    @joe.edwards said:
    I thought we had something special!

    TRWTF is not configuring the content expiration on your big sig banners so they don't redownload every time a page is refreshed.

    Maybe we should all chip in and fund a community CloudFront account for the signatures.*

    * I'm just kidding, what a bunch of commies

    I explicitly configured them to expire immediately so that it would show a different message every time.

    Clearly this should be done in javascript. Or Flash.


  • Discourse touched me in a no-no place

    @bgodot said:

    Note the second && which should be a ||
    Wrong WTF. TRWTF is note the fact that != is not defined in terms of == (or vice versa.)


  • Discourse touched me in a no-no place

    @Ronald said:

    Clearly this should be done in javascript.
    There are more evil alternatives. And some ways of using Javascript that maximise the vile evilness.@Ronald said:
    Or Flash.
    [Golf clap]



  • @mikeTheLiar said:

    @joe.edwards said:
    @Mo6eB said:
    How the unholy fuck did you format your code like that? Manually in the html editor? In some other code-formatting tool that spits out html? Tell me your secrets, wise one.
    It's an ancient Cantonese secret.
    I'm prefer the ancient Russian secret.
     

    Well I can finally contribute to this thread then! Code has been slightly anonymised due to me not having a copy any more and not remembering the names:

    class Base {
        int fieldA;
    public:
        Base(int a) : fieldA(a) {}
        Base(Base const& other) : fieldA(other.fieldA) {}
        bool operator== (Base const& other) {
            return fieldA == other.fieldA;
        }
    };
    

    class Derived : public Base {
    int fieldB;
    public:
    Derived(int a, int b) : Base(a), fieldB(b) {}
    Derived(Base const& other) : Base(other), fieldB(0) {}
    bool operator== (Derived const& other) {
    if (fieldB == 0 || other.fieldB == 0) {
    throw "You didn't initialise fieldB, you are a bad coder, your code is bad and you should feel bad. fuck you";
    }
    return fieldA == other.fieldA && fieldB == other.fieldB;
    }
    };

    int main(int argc, char** argv) {
    Base base(5);
    Derived derived(5, 6);
    base == derived; // returns true
    derived == base; // throws exception
    return 0;
    }

    For added fun, turned out parts of the system relied on that exception being thrown to detect malconstructed objects and I could fix neither the constructor, nor the operator


  • Discourse touched me in a no-no place

    @Mo6eB said:

        base == derived; // returns true
    derived == base; // throws exception
    Oh that's just peachy. And people wonder why C++ is an evil language.@Mo6eB said:
    For added fun, turned out parts of the system relied on that exception being thrown to detect malconstructed objects and I could fix neither the constructor, nor the operator
    Ugh. Figures…


Log in to reply