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

Add strong and weak ETag comparisons #695

Merged
merged 1 commit into from
Aug 30, 2016
Merged

Add strong and weak ETag comparisons #695

merged 1 commit into from
Aug 30, 2016

Conversation

JunTaoLuo
Copy link
Contributor

The implementation of Equals simply checks whether two ETags are equivalent but does not comform to the RFC's specifications in terms of comparing ETags listed here: https://tools.ietf.org/html/rfc7232#section-2.3.2.

cc @Tratcher

@@ -103,7 +103,7 @@ public override bool Equals(object obj)
}

// Since the tag is a quoted-string we treat it case-sensitive.
return ((_isWeak == other._isWeak) && (string.CompareOrdinal(_tag, other._tag) == 0));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any reason we were going in a roundabout way of comparing strings previously? Why CompareOrdinal instead of Equals?

Copy link
Member

Choose a reason for hiding this comment

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

If you're going to use string.Equals you still need to pass in Ordinal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The default is an ordinal comparison.

Copy link
Member

Choose a reason for hiding this comment

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

Always specify for string operations, the defaults are not consistant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting... How and when do they vary?

Copy link
Contributor

Choose a reason for hiding this comment

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

When then stars align ...

Also MSDN recommended use overload without default option.

We recommend that you select an overload that does not use default values, for the following reasons:

  • Some overloads with default parameters (those that search for a Char in the string instance) perform an ordinal comparison, whereas others (those that search for a string in the string instance) are culture-sensitive. It is difficult to remember which method uses which default value, and easy to confuse the overloads.
  • The intent of the code that relies on default values for method calls is not clear. In the following example, which relies on defaults, it is difficult to know whether the developer actually intended an ordinal or a linguistic comparison of two strings, or whether a case difference between protocol and "http" might cause the test for equality to return false.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fascinating, didn't know the defaults can vary in such ways. I guess it's always better to be on the safe side and just be explicit in what you want to do haha.

Copy link
Contributor

Choose a reason for hiding this comment

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

Being explicit greatly improves readability as it removes confusions.

@JunTaoLuo JunTaoLuo mentioned this pull request Aug 30, 2016
@JunTaoLuo
Copy link
Contributor Author

🆙📅

@Tratcher
Copy link
Member

:shipit:

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

Successfully merging this pull request may close these issues.

5 participants