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

[API Proposal]: Consider support for IEquatable<Uri> on System.Uri #97940

Open
dahlia opened this issue Feb 4, 2024 · 9 comments
Open

[API Proposal]: Consider support for IEquatable<Uri> on System.Uri #97940

dahlia opened this issue Feb 4, 2024 · 9 comments
Labels
api-ready-for-review API is ready for review, it is NOT ready for implementation area-System.Net
Milestone

Comments

@dahlia
Copy link

dahlia commented Feb 4, 2024

Edited by @MihaZupan on 2024-02-16

Background and motivation

For a long time, the System.Uri type has provided equality comparison operations via the object.Equals(object?) method and the operator ==() & operator !=() operators. Therefore, there is no reason not to further implement the IEquatable<Uri> interface.

API Proposal

namespace System;

-public partial class Uri : ISpanFormattable, ISerializable
+public partial class Uri : ISpanFormattable, ISerializable, IEquatable<Uri>
{
    // Existing
    public override bool Equals(object? comparand);

+   public bool Equals(Uri? other);
}

API Usage

IEquatable<Uri> a = new Uri("https://example.com/a");
IEquatable<Uri> b = new Uri("https://example.com/b");
Console.WriteLine("Equals? {0}", a.Equals(b));

Alternative Designs

No response

Risks

No response

@dahlia dahlia added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Feb 4, 2024
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Feb 4, 2024
@ghost
Copy link

ghost commented Feb 4, 2024

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

For a long time, the System.Uri type has provided equality comparison operations via the object.Equals(object?) method and the operator ==() & operator !=() operators. Therefore, there is no reason not to further implement the IEquatable<Uri> interface.

API Proposal

namespace System;

public class Uri : IEquatable<Uri>, ...
{
    bool IEquatable<Uri>.Equals(Uri other);

    ...
}

API Usage

IEquatable<Uri> a = new Uri("https://example.com/a");
IEquatable<Uri> b = new Uri("https://example.com/b");
Console.WriteLine("Equals? {0}", a.Equals(b));

Alternative Designs

No response

Risks

No response

Author: dahlia
Assignees: -
Labels:

api-suggestion, area-System.Net, untriaged

Milestone: -

@dahlia dahlia changed the title [API Proposal]: Consider support for IEquatable<Uri> on System.Uri [API Proposal]: Consider support for IEquatable<Uri> on System.Uri Feb 4, 2024
@teo-tsirpanis
Copy link
Contributor

I don't see a benefit here; IEquatable<T> is particularly useful in value types to avoid boxing.

@RenderMichael
Copy link
Contributor

This would allow Uris to participate in certain generic methods like MemoryExtensions.SequenceEqual.

Api Usage

bool AreEqual(Uri[] left, Uri[] right)
{
    return left.AsSpan().SequenceEqual(right.AsSpan());
}

@dahlia
Copy link
Author

dahlia commented Feb 5, 2024

If implementing IEquatable<T> adds no benefit to reference types, why does System.String implement IEquatable<String>? I'm just curious.

@huoyaoyuan
Copy link
Member

Dictionary requires IEquatable<T> for default value equality. For cases where an equality comparer is optional, implementing IEquatable<T> can change the behavior of fallback comparer.

@MichalPetryka
Copy link
Contributor

Dictionary requires IEquatable<T> for default value equality.

Isn't overriding the default Equals enough for that?

@MihaZupan
Copy link
Member

This seems reasonable to me. The main benefits I see are:

  • being able to call methods that are limited to IEquatable (e.g. spanOfUris.Contains(uri))
  • having an overload of Equals on Uri that takes a strongly typed Uri instead of just object
    • The implementation shouldn't be explicit, we do want it to show up in IntelliSense for Uri
namespace System;

-public partial class Uri : ISpanFormattable, ISerializable
+public partial class Uri : ISpanFormattable, ISerializable, IEquatable<Uri>
{
    // Existing
    public override bool Equals(object? comparand);

+   public bool Equals(Uri? other);
}

@dotnet/ncl any thoughts against marking this one as ready for review?

@MihaZupan MihaZupan added this to the Future milestone Feb 16, 2024
@MihaZupan MihaZupan removed the untriaged New issue has not been triaged by the area owner label Feb 16, 2024
@stephentoub
Copy link
Member

Seems fine.

@MihaZupan MihaZupan added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-ready-for-review API is ready for review, it is NOT ready for implementation area-System.Net
Projects
None yet
Development

No branches or pull requests

8 participants