Skip to content

[BUG]: StringEnum equality comparison is suprising #738

@danielmarbach

Description

@danielmarbach

What happened?

The following test fails which is surprising

    [Theory]
    [InlineData("App", HookType.App)]
    [InlineData("Organization", HookType.Organization)]
    [InlineData("Repository", HookType.Repository)]
    public void CanCompareToEnumValue(string stringValue, HookType enumValue)
    {
        var stringEnum = new StringEnum<HookType>(stringValue);
        stringEnum.Should().Be((StringEnum<HookType>)enumValue);

The problem is that StringEnum is a record type. Enums 's can get implicitly converted to StringEnum and depending on whether the internal parsed value is already set or not the structural comparison of the record returns different results.

StringEnum should probably override structural comparison (and the hash code generation too). An equal implementation could look like

    public bool Equals(StringEnum<TEnum>? other)
    {
        if (other is null)
        {
            return false;
        }

        var result = this.TryParse(out var value);
        var otherResult = other.TryParse(out var otherValue);
        return result && otherResult && value.Equals(otherValue);
    }

but then it is questionable whether it is still useful to declare the type as a record. If the change would have been more straightforward I would have submitted a PR. Still happy to contribute once some direction is discussed / given.

Versions

Octokit.Webhooks 3.0.0

Relevant log output

Code of Conduct

  • I agree to follow this project's Code of Conduct

Metadata

Metadata

Assignees

No one assigned

    Labels

    Status: TriageThis is being looked at and prioritizedType: BugSomething isn't working as documented

    Type

    No type

    Projects

    Status

    🆕 Triage

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions