tags:

views:

12379

answers:

7

I need to do a comparaison between an object and NULL. When the object is not NULL I fill it with some data.

Here is the code :

 if (region != null)
 {
  ....
 }

This is working but when looping and looping sometime the region object is NOT null (I can see data inside it in debug mode). In step-by-step when debugging, it doesn't go inside the IF statement... When I do a Quick Watch with these following expression : I see the (region == null) return false, AND (region != null) return false too... why and how?

Update

Someone point out that the object was == and != overloaded:

    public static bool operator ==(Region r1, Region r2)
    {
        if (object.ReferenceEquals(r1, null))
        {
            return false;
        }
        if (object.ReferenceEquals(r2, null))
        {
            return false;
        }

        return (r1.Cmr.CompareTo(r2.Cmr) == 0 && r1.Id == r2.Id);
    }


    public static bool operator !=(Region r1, Region r2)
    {
        if (object.ReferenceEquals(r1, null))
        {
            return false;
        }
        if (object.ReferenceEquals(r2, null))
        {
            return false;
        }
        return (r1.Cmr.CompareTo(r2.Cmr) != 0 || r1.Id != r2.Id);
    }
+1  A: 

This can sometimes happen when you have multiple threads working with the same data. If this is the case, you can use a lock to prevent them from messing with eachother.

skb
Thx, but the reason is really the == and != that wasn't overloaded well. But your answer could have been correct.
Daok
+15  A: 

Is the == and/or != operator overloaded for the region object's class?

Now that you've posted the code for the overloads:

The overloads should probably look like the following (code taken from postings made by Jon Skeet and Philip Rieck):

public static bool operator ==(Region r1, Region r2)
{
    if (object.ReferenceEquals( r1, r2)) {
        // handles if both are null as well as object identity
        return true;
    }

    if ((object)r1 == null || (object)r2 == null)
    {
       return false;
    }        

    return (r1.Cmr.CompareTo(r2.Cmr) == 0 && r1.Id == r2.Id);
}

public static bool operator !=(Region r1, Region r2)
{
    return !(r1 == r2);
}
Michael Burr
I just realized that the I have == and != overloaded!
Daok
I think someone made a mistake in the != part, I'll check that! Thx for the hint.
Daok
Correct, the overloads are wrong - if you copy in the code or explanation from my post below I'll delete my post
Philip Rieck
@Philip Rieck - Done, though I'm not sure you need to remove your post (and I hope I didn't botch the code logic).
Michael Burr
+8  A: 

Those operator overloads are broken.

Firstly, it makes life a lot easier if != is implemented by just calling == and inverting the result.

Secondly, before the nullity checks in == there should be:

if (object.ReferenceEquals(r1, r2))
{
    return true;
}
Jon Skeet
OK I'll do the modification in few minutes.
Daok
A: 

So is it that these checks here are not right:

public static bool operator !=(Region r1, Region r2)
{
    if (object.ReferenceEquals(r1, null))
    {
        return false;
    }
    if (object.ReferenceEquals(r2, null))
    {
        return false;
    }
...
dlamblin
Yes - you need an extra check beforehand, to account for the case where r1 and r2 are both null. Check my answer.(This also optimises the case where r1 and r2 are both non-null, but refer to the same object.)
Jon Skeet
+4  A: 

Both of the overloads are incorrect

 public static bool operator ==(Region r1, Region r2)
    {
        if (object.ReferenceEquals(r1, null))
        {
            return false;
        }
        if (object.ReferenceEquals(r2, null))
        {
            return false;
        }

        return (r1.Cmr.CompareTo(r2.Cmr) == 0 && r1.Id == r2.Id);
    }

if r1 And r2 are null, the first test (object.ReferenceEquals(r1, null)) will return false, even though r2 is also null.

try

//ifs expanded a bit for readability
 public static bool operator ==(Region r1, Region r2)
    {
        if( (object)r1 == null && (object)r2 == null)
        {
           return true;
        }
        if( (object)r1 == null || (object)r2 == null)
        {
           return false;
        }        
        //btw - a quick shortcut here is also object.ReferenceEquals(r1, r2)

        return (r1.Cmr.CompareTo(r2.Cmr) == 0 && r1.Id == r2.Id);
    }
Philip Rieck
I am testing. give me 2 min
Daok
This is working!!!!!!!! Thx you would have been my "Accepted answer" but someone else find the why first and you got the how :)
Daok
A: 

There's another possibility that you need to click the refresh icon next to the parameter that you're watching. VS try to keep up with the performance while not evaluating every statement/parameter. Take a look to make sure, before you start making changes to places that's non relevant.

faulty
A: 

For equality comparison of a type "T", overload these methods:

int GetHashCode() //Overrides Object.GetHashCode
bool Equals(object other) //Overrides Object.Equals; would correspond to IEquatable, if such an interface existed
bool Equals(T other) //Implements IEquatable<T>; do this for each T you want to compare to
static bool operator ==(T x, T y)
static bool operator !=(T x, T y)

Your type-specific comparison code should be done in one place: the type-safe IEquatable<T> interface method Equals(T other). If you're comparing to another type (T2), implement IEquatable<T2> as well, and put the field comparison code for that type in Equals(T2 other).

All overloaded methods and operators should forward the equality comparison task to the main type-safe Equals(T other) instance method, such that an clean dependency hierarchy is maintained and stricter guarantees are introduced at each level to eliminate redundancy and unessential complexity.

bool Equals(object other)
{
    if (other is T) //replicate this for each IEquatable<T2>, IEquatable<T3>, etc. you may implement
     return Equals( (T)other) ); //forward to IEquatable<T> implementation
    return false; //other is null or cannot be compared to this instance; therefore it is not equal
}

bool Equals(T other)
{
    if ((object)other == null) //cast to object for reference equality comparison, or use object.ReferenceEquals
     return false;
    //if ((object)other == this) //possible performance boost, ONLY if object instance is frequently compared to itself! otherwise it's just an extra useless check
     //return true;
    return field1.Equals( other.field1 ) &&
           field2.Equals( other.field2 ); //compare type fields to determine equality
}

public static bool operator ==( T x, T y )
{
    if ((object)x != null) //cast to object for reference equality comparison, or use object.ReferenceEquals
     return x.Equals( y ); //forward to type-safe Equals on non-null instance x
    if ((object)y != null)
     return false; //x was null, y is not null
    return true; //both null
}

public static bool operator !=( T x, T y )
{
    if ((object)x != null)
     return !x.Equals( y ); //forward to type-safe Equals on non-null instance x
    if ((object)y != null)
     return true; //x was null, y is not null
    return false; //both null
}

Discussion:

The preceding implementation centralizes the type-specific (i.e. field equality) comparison to the end of the IEquatable<T> implementation for the type. The == and != operators have a parallel but opposite implementation. I prefer this over having one reference the other, such that there is an extra method call for the dependent one. If the != operator is simply going to call the == operator, rather than offer an equally performing operator, then you may as well just use !(obj1 == obj2) and avoid the extra method call. The comparison-to-self is left out from the equals operator and the IEquatable<T> implementations, because it can introduce 1. unnecessary overhead in some cases, and/or 2. inconsistent performance depending on how often an instance is compared to itself vs other instances.

An alternative I don't like, but should mention, is to reverse this setup, centralizing the type-specific equality code in the equality operator instead and have the Equals methods depend on that. One could then use the shortcut of ReferenceEquals(obj1,obj2) to check for reference equality and null equality simultaneously as Philip mentioned in an earlier post, but that idea is misleading. It seems like you're killing two birds with one stone, but your actually creating more work -- after determining the objects are neither both null nor the same instance, you will then, in addition, STILL have to on to check whether each instance is null. In my implementation, you check for any single instance being null exactly once. By the time the Equals instance method is called, it's already ruled out that the first object being compared is null, so all that's left to do is check whether the other is null. So after at most two comparisons, we jump directly into the field checking, no matter which method we use (Equals(object),Equals(T),==,!=). Also, as I mentioned, if you really are comparing and object to itself the majority of the time, then you could add that check in the Equals method just before diving into the field comparisons. The point in adding it last is that you can still maintain the flow/dependency hierarchy without introducing a redundant/useless check at every level.

Triynko