11

I'm getting this warning but can't figure out the problem...

CodeContracts: warning: The Boolean condition d1.Count != d2.Count always evaluates to a constant value. If it (or its negation) appear in the source code, you may have some dead code or redundant check

The code is as follows:

public static bool DictionaryEquals<TKey, TValue>(IDictionary<TKey, TValue> d1, IDictionary<TKey, TValue> d2)
{
    if (d1 == d2) return true;
    if (d1 == null || d2 == null) return false;
    if (d1.Count != d2.Count) return false; // <-- warning here

    // Equality check goes here

    return true;
}

The // Equality check goes here part can be as is, or replaced by a proper implementation and I still get the same warning.

Community
  • 1
  • 1
antak
  • 19,481
  • 9
  • 72
  • 80
  • Seems that '(d1 == d2)' should be changed into `Object.ReferenceEquals(d1, d2)` – Dmitry Bychenko Feb 11 '15 at 11:14
  • @DmitryBychenko That does the same thing. There is no overloaded operator for `IDictionary`. –  Feb 11 '15 at 11:15
  • @hvd You can't provide operator overload for interfaces – Sriram Sakthivel Feb 11 '15 at 11:16
  • @SriramSakthivel Yeah, I realised that after commenting, and have already edited my comment. :) –  Feb 11 '15 at 11:17
  • `Count` is inherited from `ICollection>`, and changing `DictionaryEquals` to `CollectionEquals` causes the same warning. (This doesn't explain anything, it just gives a slightly simpler example to reproduce the problem.) –  Feb 11 '15 at 11:35
  • @DmitryBychenko Yeah, I thought that might be it, but I get the same warning after replacing with `ReferenceEquals(d1, d2)`.. Besides, I can't reason why CodeContract should warn even if `d1 == d2` meant something else. – antak Feb 11 '15 at 12:25
  • Are you ensuring that `d1.Count == d2.Count` elsewhere in your code before this method is called, or perhaps this is always the case based on how the code is written? If so, then the `d1.Count != d2.Count` condition will always evaluate to false, and thus the contract warning is correct. (It would help to see the calling code.) – Keith Feb 11 '15 at 14:14
  • @Keith FWIW, I tried making an copy of the method that's called from nowhere and I still get the same warning. Otherwise, the caller is really quite complex and not easily pastable. (Passed arguments are instances created at various stages of runtime and not some literal.) – antak Feb 11 '15 at 14:26

1 Answers1

5

This is simply a bug in Code Contracts. It is easy to concoct inputs that make this condition true or false. The warning is bogus.

From personal experience I know that bugs in CC are not rare.

How to fix? Since this is a bug there is no official/intended course of action. Report the bug. Jiggle the code around until the warning goes away (for example, try ReferenceEquals which is better style anyway). Suppress the warning. Things like that.

usr
  • 168,620
  • 35
  • 240
  • 369
  • Is this a known bug or are you speculating? Can you link to an official bug report? – Keith Feb 11 '15 at 14:16
  • 1
    @Keith how could it not be a bug? The statement made in that warning is clearly false. – usr Feb 11 '15 at 14:19
  • I'm not yet convinced that it is a bug. See my comment to the OP above. The calling code could be written in a way that ensures the condition is always false. – Keith Feb 11 '15 at 14:25
  • @Keith it could but it does not have to. The caller can make `d1.Count != d2.Count` an arbitrary boolean. Do you agree? Therefore the statement that this expression is constant is false. – usr Feb 11 '15 at 14:28
  • 1
    @Keith I think I see your point now. CC is not sensitive to cross-method data flow. It does not evaluate what the caller does. Methods are checked in isolation. – usr Feb 11 '15 at 14:43
  • +1 _From personal experience I know that bugs in CC are not rare_ Thanks, I wish I'd known how fallible it was before wasted hours on it. – antak Feb 14 '15 at 10:15