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

Add proposal for 'key' modifier #3127

Closed
wants to merge 1 commit into from
Closed

Conversation

agocke
Copy link
Member

@agocke agocke commented Jan 27, 2020

Implements structural equality.

Implements structural equality.
protected bool KeyEquals(A a)
=> P1 == a.P1;

public static bool operator==(A left, A right) => left.Equals(right);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't check for null?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! Yeah, it seems good to make this null-aware.

```


Three synthesized members will be added to the type, `T`:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is well more than 3 synthesized members.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heh, just made up a number before writing this list and forgot to go back


Three synthesized members will be added to the type, `T`:

1. override of `object.Equals(object)`. It is an error if a base class has sealed this member.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also consider commenting on what happens if the class has written out implementations of any of these members.

@agocke agocke marked this pull request as ready for review January 28, 2020 17:57
@omariom
Copy link

omariom commented Feb 13, 2020

EqualityContractOrigin is basically GetType(), right?

Then B.Equals could be:

public bool Equals(B other)  => other.GetType() == typeof(B) && KeyEquals(b);

obj.GetType() == typeof(SomeType) is optimised by JIT since probably .NET 2.0 and is extremely efficient (not sure about other runtimes). Using EqualityContractOrigin means Type objects will have to be instantiated.

@CyrusNajmabadi
Copy link
Member

EqualityContractOrigin is basically GetType(), right?

No. It can be implemented that way, but it's not required to be implemented that way. The point is to allow the impl (including user overrides) to make the call on what determines equality.

@gafter
Copy link
Member

gafter commented Apr 7, 2020

EqualityContractOrigin is basically GetType(), right?

No. Having it be a virtual method means that you can extend a class (implementation inheritance) without changing its equality contract.

@omariom
Copy link

omariom commented Apr 8, 2020

Can Equals be implemented in a more efficient manner if B is sealed?
What if B already has its own implementation of IEquatable<B>?

Base automatically changed from master to main March 12, 2021 19:15
@agocke agocke closed this Jun 20, 2021
@agocke agocke deleted the key-proposal branch June 20, 2021 19:06
@bugproof
Copy link

Why closed?

@agocke
Copy link
Member Author

agocke commented Jun 24, 2021

We ended up implementing records without the notion and at the moment I don't think it's worth the additional complexity.

@bugproof
Copy link

I think it would still be useful for records in case you wanted to compare only by certain properties and not all.

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

Successfully merging this pull request may close these issues.

7 participants