45

Today I stumbled upon an interesting bug I wrote. I have a set of properties which can be set through a general setter. These properties can be value types or reference types.

public void SetValue( TEnum property, object value )
{
    if ( _properties[ property ] != value )
    {
        // Only come here when the new value is different.
    }
}

When writing a unit test for this method I found out the condition is always true for value types. It didn't take me long to figure out this is due to boxing/unboxing. It didn't take me long either to adjust the code to the following:

public void SetValue( TEnum property, object value )
{
    if ( !_properties[ property ].Equals( value ) )
    {
        // Only come here when the new value is different.
    }
}

The thing is I'm not entirely satisfied with this solution. I'd like to keep a simple reference comparison, unless the value is boxed.

The current solution I am thinking of is only calling Equals() for boxed values. Doing a check for a boxed values seems a bit overkill. Isn't there an easier way?

Community
  • 1
  • 1
Steven Jeuris
  • 18,274
  • 9
  • 70
  • 161
  • Surely if you want different behaviour for boxed values then you're going to need to check whether you're dealing with a boxed value? – LukeH Jun 01 '11 at 17:19
  • Make a generic overload of this method with type T where T : struct – Lukasz Madon Jun 01 '11 at 17:23
  • 1
    @lukas, will not work unless there is more of a difference than merely the `T` and a constraint. – Anthony Pegram Jun 01 '11 at 17:29
  • What I mean by that: split it in two methods. One is dealing with ref types, other one with value types and correspoding logic in each function. @Steven I've an idea but it's bloody dengerous.. I mean I didn't try it out:P If you are using C# 4.0, use dynamic insted of object. I assume you got some performance reason to do what you do but it maight be good enough. – Lukasz Madon Jun 01 '11 at 18:21
  • Related posts - [Equality of two structs in C#](https://stackoverflow.com/q/2042282/465053) *&* ['Object.ReferenceEquals' is always false because it is called with a value type](https://stackoverflow.com/q/26661602/465053) – RBT Jun 15 '18 at 13:46
  • Another related post - [Are value types immutable by definition?](https://stackoverflow.com/q/868411/465053) – RBT Jun 16 '18 at 03:14

5 Answers5

29

If you need different behaviour when you're dealing with a value-type then you're obviously going to need to perform some kind of test. You don't need an explicit check for boxed value-types, since all value-types will be boxed** due to the parameter being typed as object.

This code should meet your stated criteria: If value is a (boxed) value-type then call the polymorphic Equals method, otherwise use == to test for reference equality.

public void SetValue(TEnum property, object value)
{
    bool equal = ((value != null) && value.GetType().IsValueType)
                     ? value.Equals(_properties[property])
                     : (value == _properties[property]);

    if (!equal)
    {
        // Only come here when the new value is different.
    }
}

( ** And, yes, I know that Nullable<T> is a value-type with its own special rules relating to boxing and unboxing, but that's pretty much irrelevant here.)

LukeH
  • 263,068
  • 57
  • 365
  • 409
  • Thanks, this seems to work perfectly and has no major overhead. I see no increase in average runtime. – Steven Jeuris Jun 02 '11 at 20:51
  • The overhead is in boxing also calling getType() has a cost. Best to just avoid it by generating the getter and setter delegate on the fly.Keep things to actual type never boxing. – RJ Thompson Nov 03 '17 at 17:01
12

Equals() is generally the preferred approach.

The default implementation of .Equals() does a simple reference comparison for reference types, so in most cases that's what you'll be getting. Equals() might have been overridden to provide some other behavior, but if someone has overridden .Equals() in a class it's because they want to change the equality semantics for that type, and it's better to let that happen if you don't have a compelling reason not to. Bypassing it by using == can lead to confusion when your class sees two things as different when every other class agrees that they're the same.

Sean U
  • 6,730
  • 1
  • 24
  • 43
  • The problem is exactly that `Equals` might have been overridden. Although two objects are equal, that doesn't mean a new object (with a different reference) isn't set. – Steven Jeuris Jun 01 '11 at 18:03
1

Since the input parameter's type is object, you will always get a boxed value inside the method's context.

I think your only chance is to change the method's signature and to write different overloads.

Simone
  • 11,655
  • 1
  • 30
  • 43
1

How about this:

if(object.ReferenceEquals(first, second)) { return; }
if(first.Equals(second)) { return; }

// they must differ, right?

Update

I realized this doesn't work as expected for a certain case:

  • For value types, ReferenceEquals returns false so we fall back to Equals, which behaves as expected.
  • For reference types where ReferenceEquals returns true, we consider them "same" as expected.
  • For reference types where ReferenceEquals returns false and Equals returns false, we consider them "different" as expected.
  • For reference types where ReferenceEquals returns false and Equals returns true, we consider them "same" even though we want "different"

So the lesson is "don't get clever"

default.kramer
  • 5,943
  • 2
  • 32
  • 50
  • if the value was just boxed, as in the case of Value types, the first `if` will always yield false, thus it won't solve OP problem. – Simone Jun 01 '11 at 17:34
  • 1
    The problem, as I understand it, is "I'd like to keep a simple reference comparison, unless the value is boxed." This would do that. But as Sean's answer says, "if someone has overridden .Equals() in a class it's because they want to change the equality semantics for that type, and it's better to let that happen if you don't have a compelling reason not to". I assumed the asker has a compelling reason. – default.kramer Jun 01 '11 at 17:36
  • However, I will have to verify whether this is correct some other time, got to go now. I believe Simone makes a valid point. – Steven Jeuris Jun 01 '11 at 18:11
  • 3
    You've just duplicated the functionality of the built-in [`object.Equals(x,y)`](http://msdn.microsoft.com/en-us/library/w4hkze5k.aspx) static method. This basically checks for reference equality first, and if that fails it calls the polymorphic [`x.Equals(y)`](http://msdn.microsoft.com/en-us/library/bsc2ak47.aspx) instance method. – LukeH Jun 01 '11 at 23:17
0

I suppose

I'd like to keep a simple reference comparison, unless the value is boxed.

is somewhat equivalent to

If the value is boxed, I'll do a non-"simple reference comparison".

This means the first thing you'll need to do is to check whether the value is boxed or not.

If there exists a method to check whether an object is a boxed value type or not, it should be at least as complex as that "overkill" method you provided the link to unless that is not the simplest way. Nonetheless, there should be a "simplest way" to determine if an object is a boxed value type or not. It's unlikely that this "simplest way" is simpler than simply using the object Equals() method, but I've bookmarked this question to find out just in case.

(not sure if I was logical)

blizpasta
  • 2,624
  • 3
  • 25
  • 31