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

Default implementation of GetHashCode on records includes mutable fields #912

Closed
allykzam opened this issue Jan 29, 2016 · 2 comments
Closed

Comments

@allykzam
Copy link

The current default implementation for GetHashCode on record types currently includes mutable fields when calculating the result. Working with a WPF ListView and binding the ItemsSource property to an ObservableCollection<T> containing records with mutable fields, I found that if a selection in the list is made, and then any of the mutable fields in those selected values are modified, the list no longer allows de-selection of the corresponding ListViewItem elements. The ListView.SelectedItems collection still appears to contain those values, and refuses to deselect those elements either programmatically or via the mouse/keyboard by clicking other things in the list.

The MSDN documentation on the Object.GetHashCode states:

You can override GetHashCode for immutable reference types. In general, for mutable reference types, you should override GetHashCode only if:

  • You can compute the hash code from fields that are not mutable; or
  • You can ensure that the hash code of a mutable object does not change while the object is contained in a collection that relies on its hash code.

Otherwise, you might think that the mutable object is lost in the hash table. If you do choose to override GetHashCode for a mutable reference type, your documentation should make it clear that users of your type should not modify object values while the object is stored in a hash table.

As a record with mutable fields isn't fully immutable anymore, my view is that the default GetHashCode implementation should meet the given bullet points (unless there's some clear documentation to the contrary somewhere I haven't seen yet?). It currently violates both bullet points.

This can currently be worked around by overriding GetHashCode, although this means also adding the CustomEquality and CustomComparison attributes, overriding Equals(obj), and implementing either IComparable or IStructuralComparable. Since it is only an issue when using records with mutable fields and the workaround is short enough, I assume this won't get a very high priority, but is it possible to address this in the future?

For what it's worth, this appears to be tangentially-related to this UserVoice poist, although the desired changes are very different.

@dsyme
Copy link
Contributor

dsyme commented Jan 29, 2016

As mentioned in CONTRIBUTING.md, please add to http://fslang.uservoice.com as this is a feature request (or a design-change request)

@dsyme dsyme closed this as completed Jan 29, 2016
@allykzam
Copy link
Author

Posted here as it behaves like a bug (the default GetHashCode disobeys MSDN's suggestions on how to implement it, and doesn't mention this anywhere I've seen), but I'll re-post on UserVoice shortly.

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

No branches or pull requests

2 participants