0

I'm trying to compare objects within two different Dictionary<string,object> (to find differences in a versioned repository.

The dictionary can contains any serialize type, either value-type or reference-type.

I loop on all keys to perform the comparison. Because of the value type boxing, I implemented a small utility method found on SO :

private static bool AreValueEquals(object o1, object o2)
{
    return (o1 != null && o1.GetType().IsValueType)
        ? o1.Equals(o2)
        : o1 == o2;
}

This method is used in my main method like this:

private static List<string> GetDifferent(Dictionary<string, object> currentValues, Dictionary<string, object> previousValues)
{
    var changed = from fieldName in currentValues.Keys.Union(previousValues.Keys).Distinct()
                  let currentVal = GetIfExists(currentValues, fieldName)
                  let previousVal = GetIfExists(previousValues, fieldName)
                  where !AreValueEquals(currentVal, previousVal)
                  select fieldName;
    return changed.ToList();
}
private static object GetIfExists(Dictionary<string, object> values, string fieldName)
{
    return values.ContainsKey(fieldName) ? values[fieldName] : null;
}

While the AreValueEquals method works as expected on my test case (dotnetfiddle), at runtime, I get unexpected result:

result

I don't understand this result. Is my implementation correct? How to fix?

Community
  • 1
  • 1
Steve B
  • 36,818
  • 21
  • 101
  • 174
  • Don't roll your own, the framework provides a [`Comparer.Default`](https://msdn.microsoft.com/en-us/library/azhsac5f%28v=vs.110%29.aspx) comparer for this purpose. If you can't use that, please explain your **unique** requirement. But this is what the `Dictionary` itself uses. – Ben Voigt May 04 '15 at 21:00
  • I don't know the type at compile time... only at runtime. I cannot use this generic class, isn't it? – Steve B May 04 '15 at 21:01
  • It implements the non-generic `IComparer` interface. You need a little reflection (or the `dynamic` keyword) to get the right comparer. – Ben Voigt May 04 '15 at 21:03
  • Actually, just `bool AreValueEquals(dynamic o1, dynamic o2) { return o1 == o2; }` might be your easiest approach. – Ben Voigt May 04 '15 at 21:04
  • Just tried `Comparer.Default.Compare(o1,o2)==0` in my test case... Surprisingly, it seems to work as expected. – Steve B May 04 '15 at 21:05
  • oops, I should have said [`EqualityComparer.Default`](https://msdn.microsoft.com/en-us/library/ms224763(v=vs.110).aspx) not `Comparer.Default` – Ben Voigt May 04 '15 at 21:08
  • I don't see why you would need that utility method. Static `object.Equals` would work just fine for your case. – Mike Zboray May 04 '15 at 21:37

2 Answers2

1

String is a reference type.

I don't know how you are creating those strings but they a re probably represented as 2 different instances of string object.

In you method you aare doing a == on 2 objects. This will by default check only if they are the same references.

Why not use Generics and use the Comparer.Default or just use the Equals() with a null check considering your are boxing?

    object a = "d";
    object b = new String(new []{'d'});

    Console.Write("a == b: ");

    Console.WriteLine(a == b);

    Console.WriteLine("AreValueEquals: " + AreValueEquals(a,b));

    Console.WriteLine("Equals: " + a.Equals(b));

Gives:

a == b: False
AreValueEquals: False
Equals: True

Confirming the intern:

    Console.WriteLine("\r\nComparing 2 constants\r\n");

    object c = "d";

    Console.Write("a == c: ");

    Console.WriteLine(a == c);

    Console.WriteLine("AreValueEquals: " + AreValueEquals(a,c));

    Console.WriteLine("Equals: " + a.Equals(c));

Gives:

Comparing 2 constants

a == c: True
AreValueEquals: True
Equals: True

Have a look at this fiddle

Michal Ciechan
  • 13,492
  • 11
  • 76
  • 118
  • if confirm your hypothesis... using the debugger, I can see that `o1 == o2` is false with strings. I actually wonder how this could have worked on .Net fiddled – Steve B May 04 '15 at 21:11
  • I have added fiddle + code from it. In the .net fiddle you were probably creating 2 strings by using constants. Compiler is smart enough to know that they are the same thing, and will intern it, and use the same interned reference. – Michal Ciechan May 04 '15 at 21:13
  • Why not just use Equals? – Michal Ciechan May 04 '15 at 21:15
  • if I wrote "x" using `new string((char)('y' -1),1)`, the comparison is false (and confirm the intern behavior) – Steve B May 04 '15 at 21:16
  • *using Equals*? do you mean `if(o1 == null && o2==null) return true; if(o1 != null) return o1.Equals(o2); return false;` ? – Steve B May 04 '15 at 21:20
  • Yep. The reason this often doesn't work is if you use value types. As you cannot check a value type for null. But as you are boxing already (very expensive) you can do the check you provided. – Michal Ciechan May 04 '15 at 21:22
  • Finally, you helped me to understand the root of my issue (string comparison, hidden by my test case which interned the constants). Thank you – Steve B May 04 '15 at 21:30
0

I've changed the GetDifferent method to what I think you are after:

private static List<string> GetDifferent(Dictionary<string, object> currentValues, Dictionary<string, object> previousValues)
{
    var changed = currentValues
                  .Where(k => previousValues.Any(p => p.Key == k.Key && !AreValueEquals(k.Value, p.Value)))
                  .Select(k => k.Key);
    return changed.ToList();
}

See this fiddle: https://dotnetfiddle.net/JZ6v6a

garryp
  • 5,508
  • 1
  • 29
  • 41