-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix #3116 instances of gethashcode in equals implementation #3117
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
Conversation
7825e2e
to
91fc730
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments
) | ||
); | ||
} | ||
|
||
private static bool EqualsAllIndices(IReadOnlyList<IndexName> indicesCurrent, IReadOnlyList<IndexName> indicesOther) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thisIndices
and otherIndices
?
if (indicesCurrent == null && indicesOther == null) return true; | ||
if (indicesCurrent == null || indicesOther == null) return false; | ||
if (indicesCurrent.Count != indicesOther.Count) return false; | ||
return indicesCurrent.Zip(indicesOther, Tuple.Create).All(t=>t.Item1.Equals(t.Item2)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should also take into account the order of indices e.g.
Indices indices1 = "foo,bar";
Indices indices2 = "bar,foo";
(indices1 == indices2).Should().BeTrue();
) | ||
); | ||
} | ||
|
||
private static bool EqualsAllTypes(IReadOnlyList<TypeName> indicesCurrent, IReadOnlyList<TypeName> indicesOther) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thisTypes
and otherTypes
?
if (indicesCurrent == null && indicesOther == null) return true; | ||
if (indicesCurrent == null || indicesOther == null) return false; | ||
if (indicesCurrent.Count != indicesOther.Count) return false; | ||
return indicesCurrent.Zip(indicesOther, Tuple.Create).All(t=>t.Item1.Equals(t.Item2)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should also take into account the order
All good points @russcam, addressed them in followup commit. |
* fix #3116 instances of gethashcode in equals implementation * Equals on types and indices now ignores order and implements == operator Conflicts: src/Nest/CommonAbstractions/Infer/RelationName/RelationName.cs src/Tests/ClientConcepts/HighLevel/Inference/TypesAndRelationsInference.doc.cs src/Tests/Framework/Configuration/Versions/ElasticsearchVersion.cs src/Tests/Framework/ElasticsearchVersionTests.cs
* fix #3116 instances of gethashcode in equals implementation * Equals on types and indices now ignores order and implements == operator Conflicts: src/Nest/CommonAbstractions/Infer/Indices/Indices.cs src/Nest/CommonAbstractions/Infer/Types/Types.cs
backported to |
No description provided.