Skip to content
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

Generated Equals for struct mapped to class in C# #2214

Closed
bernardnormier opened this issue May 22, 2024 · 1 comment · Fixed by #2235
Closed

Generated Equals for struct mapped to class in C# #2214

bernardnormier opened this issue May 22, 2024 · 1 comment · Fixed by #2235
Assignees
Milestone

Comments

@bernardnormier
Copy link
Member

bernardnormier commented May 22, 2024

In C#, the default equality (Equals) for collections is reference equality, whereas other languages (C++, Java) provide value equality (they compare the elements by value).

In Ice 3.7 and before, we attempted to emulate this value equality in C# when mapping Slice structs to C# classes, with generated Equals and GetHashCode that perform "value equality". Unfortunately this implementation is not correct.

Take for example SequenceEquals:

public static bool SequenceEquals(ICollection seq1, ICollection seq2)

It uses Equals to compare the elements, which means each collection element won't be compared by value but by reference. So it's "by value" comparison is only one level deep. There are even more issues with the DictionaryEquals implementation that assumes the entries of the two IDictionaries being compared are in the same order.

I see two possible solutions:

a) don't attempt for perform deep value comparison in C# and just use the default reference equality for collections.

That's what ZeroC.Slice does already. It's the standard/expected behavior in C#. With this solution, we could reduce the generated code by mapping Slice to record classes in C#.

The downside is this change could break existing code that relies on this non-standard value equality semantics for mapped Slice structs.

b) implement value comparison correctly for collections. We'd need to check when an element is a collection (sequence or dictionary) and then recursively compare it by value. The proper way to do that in C# is by implementing an IEqualityComparer.

See for example:

https://github.com/icerpc/icerpc-csharp/blob/5247e81134423a0114cb54a1da56a76ad7072c6b/src/IceRpc/Internal/DictionaryExtensions.cs#L5

See also #2126.

@bernardnormier bernardnormier added this to the 3.8.0 milestone May 22, 2024
@externl
Copy link
Member

externl commented May 29, 2024

I prefer (a) as it's the more expected C# behavior. Users having to update their code a bit is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants