New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use IEquatable
instead of Equals
where possible
#226
Comments
Thanks for the comprehensive repro case! I haven't had a chance to dig into the NSubstitute side of this yet, but in the meantime I thought I'd offer a couple mitigations. First, I think the duplicate code in the public override bool Equals(object obj)
{
var other = obj as CompositeKeyFail;
return (other != null) && Equals(other);
} Secondly, if you have R# (or similar) you can generate equality members. In my VS it is This doesn't fix the root of the issue you are having but hopefully will make things easier for you while using the current NSub release. |
Implementation note: I think the This has given us problems before (with related tests) so need to make sure we don't break anything. (Backup of original report repo: https://github.com/dtchepak/NSubstituteTestCaseProject) |
IEquatable
instead of Equals
IEquatable
instead of Equals
IEquatable
instead of Equals
where possible
Simple repro: namespace NSubWorkshop
{
using System;
using NSubstitute;
using Xunit;
public class IntWrap : IEquatable<IntWrap>
{
public IntWrap(int value) {
Value = value;
}
public int Value { get; set; }
public int CompareTo(IntWrap other) {
return Value.CompareTo(other.Value);
}
// Uncommenting this causes test to pass:
//public override bool Equals(object obj) {
// return Equals(obj as IntWrap);
//}
public bool Equals(IntWrap other) {
return other != null &&
Value == other.Value;
}
public override int GetHashCode() {
return -1937169414 + Value.GetHashCode();
}
}
public interface ISample
{
void Use(IntWrap value);
}
public class SampleFixture
{
[Fact]
public void Sample() {
var value1 = new IntWrap(42);
var value2 = new IntWrap(42);
var sub = Substitute.For<ISample>();
sub.Use(value1);
sub.Received().Use(value2);
}
}
} |
@dtchepak @alexandrnikitin Just want to clarify whether we still want to support this issue. As David stated above, if struct would override the So if people are following best practices, they are fully compatible with NSubstitute. Does it make sense to specifically handle scenario when type itself is inconsistent (reports different equality status depending on the method is used)? Would be also happy to see that I'm overlooking something. |
@zvirja When you put it like that I'm happier to just use |
I run in to an expected test failure when I was substituting my
Dictionary<>
based cache.The key I use for the dictionary is a composite of 2 values, and we know that the key object has to implement
IEquatable<>
in order to find the value in the dictionary again. This works:Unfortunately the substitute I used for testing this failed in this case:
After some investigation I learned that in order for the substitute to work the key objects also has to override the
Equals()
method. Overriding theEquals()
method also works in the actual dictionary.Somewhat annoying when the test fails and actual code works. I think implementing
IEquatable<>
should also work with NSubstitute.see complete code here:
https://github.com/niklasda/NSubstituteTestCaseProject
Keep up the good work
The text was updated successfully, but these errors were encountered: