Skip to content
This repository has been archived by the owner on Nov 20, 2018. It is now read-only.

#600 Refactor to Equals in Path String, Adds Regression Tests #656

Merged
merged 1 commit into from
Jun 15, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions src/Microsoft.AspNetCore.Http.Abstractions/PathString.cs
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ public string Add(QueryString other)
/// <returns>True if both PathString values are equal</returns>
public bool Equals(PathString other)
{
return string.Equals(_value, other._value, StringComparison.OrdinalIgnoreCase);
return Equals(other, StringComparison.OrdinalIgnoreCase);
}

/// <summary>
Expand All @@ -224,6 +224,10 @@ public bool Equals(PathString other)
/// <returns>True if both PathString values are equal</returns>
public bool Equals(PathString other, StringComparison comparisonType)
{
if (!HasValue && !other.HasValue)
{
return true;
}
return string.Equals(_value, other._value, comparisonType);
}

Expand All @@ -236,9 +240,9 @@ public override bool Equals(object obj)
{
if (ReferenceEquals(null, obj))
{
Copy link
Member

Choose a reason for hiding this comment

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

PathString.Empty.Equals((object)null) == false where PathString.Empty.Equals((PathString)null) == true seems inconsistent.

Copy link
Member

Choose a reason for hiding this comment

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

Return !HasValue?

return false;
return !HasValue;
}
return obj is PathString && Equals((PathString)obj, StringComparison.OrdinalIgnoreCase);
return obj is PathString && Equals((PathString)obj);
}

/// <summary>
Expand All @@ -247,7 +251,7 @@ public override bool Equals(object obj)
/// <returns>The hash code</returns>
public override int GetHashCode()
{
return (_value != null ? StringComparer.OrdinalIgnoreCase.GetHashCode(_value) : 0);
return (HasValue ? StringComparer.OrdinalIgnoreCase.GetHashCode(_value) : 0);
}

/// <summary>
Expand All @@ -258,7 +262,7 @@ public override int GetHashCode()
/// <returns>True if both PathString values are equal</returns>
public static bool operator ==(PathString left, PathString right)
{
return left.Equals(right, StringComparison.OrdinalIgnoreCase);
return left.Equals(right);
}

/// <summary>
Expand All @@ -269,7 +273,7 @@ public override int GetHashCode()
/// <returns>True if both PathString values are not equal</returns>
public static bool operator !=(PathString left, PathString right)
{
return !left.Equals(right, StringComparison.OrdinalIgnoreCase);
return !left.Equals(right);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,44 @@ public void CtorThrows_IfPathDoesNotHaveLeadingSlash()
ExceptionAssert.ThrowsArgument(() => new PathString("hello"), "value", "The path in 'value' must start with '/'.");
}

[Fact]
public void Equals_EmptyPathStringAndDefaultPathString()
Copy link
Member

Choose a reason for hiding this comment

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

Add a test for GetHashCode

{
// Act and Assert
Assert.Equal(PathString.Empty, default(PathString));
Copy link
Member

Choose a reason for hiding this comment

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

Make sure the different overloads are covered:
Assert.True(PathString.Empty == default(PathString));
Assert.True(PathString.Empty.Equals(default(PathString));

Also add variations with the arguments reversed.

Assert.Equal(default(PathString), PathString.Empty);
Assert.True(PathString.Empty == default(PathString));
Assert.True(default(PathString) == PathString.Empty);
Assert.True(PathString.Empty.Equals(default(PathString)));
Assert.True(default(PathString).Equals(PathString.Empty));
}

[Fact]
public void NotEquals_DefaultPathStringAndNonNullPathString()
{
// Arrange
var pathString = new PathString("/hello");

// Act and Assert
Assert.NotEqual(pathString, default(PathString));
}

[Fact]
public void NotEquals_EmptyPathStringAndNonNullPathString()
{
// Arrange
var pathString = new PathString("/hello");

// Act and Assert
Assert.NotEqual(pathString, PathString.Empty);
}

[Fact]
public void HashCode_CheckNullAndEmptyHaveSameHashcodes()
{
Assert.Equal(PathString.Empty.GetHashCode(), default(PathString).GetHashCode());
}

[Theory]
[InlineData(null, null)]
[InlineData("", null)]
Expand Down