Runtime: String.Equals isn't intuitive and forces us to do the OrdinalIgnoreCase thing

Created on 5 Feb 2015  Â·  44Comments  Â·  Source: dotnet/runtime

We're doing a sample and we are FOREVER doing this

if (sort.Equals("Title", StringComparison.OrdinalIgnoreCase))

I think it's time for an overload or an extension that "does the right thing." This is a 10 year old pattern and if the default is wrong (as we tell people to do this) then let's make a better method. Everyone has a version and extension that does this.

api-needs-work area-System.Runtime

Most helpful comment

Something that makes this a little bit less painful with C# 6:

using static System.StringComparer;
if (OrdinalIgnoreCase.Equals(alpha, beta))
{ 
    // ...
}

All 44 comments

I agree +10e8

+1

This drives me nuts and forces me to do multi-line if statements due to very long lines this produces. I hate that. A better and much shorter syntax would be super awesome!

the default should really be changed as most apps have to have this check

+1
Makes me crazy.

Agreed - lets help our community by making the framework use the correct pattern by default

var oic = StringComparer.OrdinalIgnoreCase;

if (oic.Equals(sort,"Title"))

I'm not pushing back against this request, but does someone have a strawman to kick off the design? The best thing I can think of is: EqualsOrdinalIgnoreCase as a method on String. Is this still too long?

Why not EqualsIgnoreCase. Is Ordinal that obvious, or needed? (although semantically it would be implied.)

@shanselman I guess that's reasonable. Personally I dislike how some operations on the string class do linguistic comparisons where as others do ordinal and it's not clear from the method name what's going on. This proposal would be making the problem worse, but perhaps we should optimize for folks that have already learned for each method if it's doing ordinal or linguistic conversions.

@ellismg I hear you. I'm trying to optimize for the 80% or even the 90% case. What is the "pit of success" option? What's the one that if folks who don't know the difference pick is more likely to be correct?

+1 EqualsIgnoreCase

@rustd Are you suggesting that ignore case should be the default? Changing defaults around this is pretty much a non-starter. We've tried similar things in the past (such as switching all operations to ordinal and invariant by default in Silverlight) but ended up backing it out for a variety of reasons. There's just too much legacy code being copied and pasted around.

I like static bool EqualsIgnoreCase(string, string) and bool EqualsIgnoreCase(string), it fits in with existing overloads static bool Equals(string, string) and bool Equals(string) which already do a ordinal comparison.

@davkean @shanselman Agree on EqualsIgnoreCase.

I did a quick scan through the String class, other methods that take a StringComparison overload include Compare, CompareOrdinal, CompareTo, StartsWith, EndsWith, IndexOf, LastIndexOf, Replace. None of them seem like a high priority for an IgnoreCase overload.

What's painfully missing is a Contains overload that accepts a comparison type. It's a bigger functionality gap than EqualsIngoreCase.

public bool EqualsIgnoreCase(string value);
public static bool EqualsIgnoreCase(string a, string b);
public bool Contains(string value, StringComparison comparisonType);

This may be a bit more of a stretch, a new IStringEquatable interface could be introduced that inherits from IEquatable. IStringEquatable would add the necessary overloads for the comparison type.

public IStringEquatable<T> : IEquatable<T>
{
    bool Equals(T other, StringComparison comparisonType);
}

The C# team is currently looking at potentially implementing pattern matching and improvements to the switch statement for C# 7.0. One source of pain has been the inability to do case insensitive comparisons in a switch statement - introducing the interface would provide a hook for case insensitive comparisons as 'syntactic sugar'. The interface could be added to the other string-like types, such as HtmlString.

I never quite understood why languages don't introduce a case-insensitive string comparison operator. It's likely that because it only applies to a string, it doesn't meet the bar required for a broadly used language feature. That said, the same was once true for String types themselves (just use an array!). Case-insensitive string comparisons are a very bread & butter development operation, and as noted above can apply to types other than System.String.

Imagine the theoretical ==~ and !=~ operators (the tilde being the approximation symbol, not bitwise complement):

public static String overload ==~(String a, String b)
public static String overload !=~(String a, String b)

if ("CoreFx" ==~ "corefx") // true

Even aside from theoretical language changes (very unlikely), the interface would allow libraries such as LINQ to leverage case-insensitive string equality natively without resorting to reflection. The same holds true for String.Compare, which strangely is missing a non-static String.Compare(string, StringComparison) overload.

I love the possible case-insensitive string operators. Until then, let's do
EqualsIgnoreCase.

On Fri, Feb 6, 2015 at 10:06 AM, Eric Bickle [email protected]
wrote:

@davkean https://github.com/davkean @shanselman
https://github.com/shanselman Agree on EqualsIgnoreCase.

public bool EqualsIgnoreCase(string value);
public static bool EqualsIgnoreCase(string a, string b);

I did a quick scan through the String class, other methods that take a
StringComparison overload include Compare, CompareOrdinal, CompareTo,
StartsWith, EndsWith, IndexOf, LastIndexOf, Replace. None of them seem
like a high priority for an IgnoreCase overload.

This may be a bit more of a stretch, a new IStringEquatable interface
could be introduced that inherits from IEquatable. IStringEquatable would
add the necessary overloads for the comparison type.

public IStringEquatable : IEquatable
{
bool Equals(T other, StringComparison comparisonType);
}

The C# team is currently looking at potentially implementing pattern
matching and improvements to the switch statement for C# 7.0. One source of
pain has been the inability to do case insensitive comparisons in a switch
statement - introducing the interface would provide a hook for case
insensitive comparisons as 'syntactic sugar'. The interface could be added
to the other string-like types, such as Char, StringBuilder, and HtmlString.

I never quite understood why languages don't introduce a case-insensitive
string comparison operator. It's likely that because it only applies to a
string the logic is that it doesn't meet the broad use needed for a general
language feature. That said, the same was once true for String types
themselves (just use an array!). Case-insensitive string comparisons are a
very bread & butter development operation, and as noted above can apply to
types other than System.String.

Imagine the theoretical ==~ and !=~ operators:

public static String overload ==~(String a, String b)
public static String overload !=~(String a, String b)

if ("CoreFx" ==~ "corefx") // true

Even aside from theoretical language changes (very unlikely), the
interface would allow libraries such as LINQ to leverage case-insensitive
string equality natively without resorting to reflection. The same holds
true for String.Compare, which strangely is missing a non-static String.Compare(string,
StringComparison) overload.

—
Reply to this email directly or view it on GitHub
https://github.com/dotnet/corefx/issues/649#issuecomment-73284160.

Scott Hanselman
Donate to fight diabetes: http://hnsl.mn/fightdiabetes

I don't like the operator. It does not really self explain what it does because "approximate equals" is not well defined and could mean many different things. Is "Hello" approximate equals to "Hello!"? The Levensthein distance is 1 so it is approximatly the same, isn't it? If it is implemented for string it will probably be available to be implemented for other types. If I implement a numeric type. is 1 ==~ 0? If im looking at numbers between 0 and 1 its really not. If I am looking at two numbers a and b which are ~10^100 and (a - b) == 1 it is approximatly 0 isnt it? Other than that if(a ==~b) could mean if( a ==~ b) and if(a == (~b)) so the syntax would not work either. (~== would work)

I don't like the operators myself for the reason mentioned, it isn't something that would be broadly used.

As someone who once used Java before moving to C#, I always create an EqualsIgnoreCase() extension method as my shorthand. I think that is the best option here along with a static version.

Just found this in CoreClr's sstring.h :)

BOOL EqualsCaseInsensitive(const SString &s) 

Should we consider both an instance and static method EqualsIgnoreCase?

The instance method is great for cases outlined in @shanselman example when one string is guaranteed to be non null. In more general cases though this isn't true and developers have to write out the more elaborated form:

if (sort != null && sort.EqualsIgnoreCase("Title"))

A static method which considers null for both arguments would be a bit more concise:

if (string.EqualsIgnoreCase(sort, "Title"))

The string type already has both instance and static methods for Equals so it would be building off of that existing pattern.

Can't folks do this now? That handles the foo is null situation.

foo?.EqualsIgnoreCase("Whatever")

On Mon, Feb 9, 2015 at 7:57 AM, Jared Parsons [email protected]
wrote:

Should we consider both an instance and static method EqualsIgnoreCase?

The instance method is great for cases outlined in @shanselman
https://github.com/shanselman example when one string is guaranteed to
be non null. In more general cases though this isn't true and developers
have to write out the more elaborated form:

if (sort != null && sort.EqualsIgnoreCase("Title")

A static method which considers null for both arguments would be a bit
more concise:

if (string.EqualsIgnoreCase(sort, "Title"))

The string type already has both instance and static methods for Equals
so it would be building off of that existing pattern.

—
Reply to this email directly or view it on GitHub
https://github.com/dotnet/corefx/issues/649#issuecomment-73533033.

Scott Hanselman
Donate to fight diabetes: http://hnsl.mn/fightdiabetes

@shanselman

That won't work as typed because it produces a bool? instead of a bool. It would need another step to convert the value to a bool

if ((foo?.EqualsIgnoreCase("Whatever")).GetValueOrDefault())

Note: the extra () around the foo?.EqualsIgnoreCase are necessary.

I think the other problem with that sample is that (foo?.EqualsIgnoreCase(bar).GetValueOrDefault()) where foo and bar are both null would produce false, whereas you might expect true to be produced (like the static string.Equals method does).

I would vote for String.Equals(string other, bool ignoreCase).

This is already used in a lot of methods in the String class. If "typing less" is the idea, this would match the 80% rule.

Honestly, EqualsIgnoreCase is very strange. That opens questions to all possible alternatives. Why not ContanisIgnoreCase, EndsWithIgnoreCase, etc. Adding a new operator adds even more questions.

What I would like to see even more is that _all_ methods in the String class follow the same pattern.

After reading more in this discussion, I'm amazed how this change, that if done like proposed is one of a kind and adds a single different method to a core API just to save some typing, is being accepted like this. Many other simple issues are suffering to pass the "won't change core types like that" review from the team.

I'm just a regular guy here and I'm not sure if this was really accepted, but I see a lot of microsoft guys agreeing on this, so that is what it looks like.

Now, I think System.String needs some improvements just like the next guy, but it seems this should be a more planned change that includes more things in a standard way for such a core type.

See, the 4th most voted question for C# stackoverflow is about the same problem in String.Contains, with 1079 votes. The same question for String.Equals has only 73 votes.

And you are already proposing adding new operators to C# just for that. That's crazy! =)

Shouldn't we standardize all methods to accept a StringComparison and a bool ignoreCase before thinking about adding new methods as shortcuts?

This thread touches on several important problems in our APIs. I think we should try to address these. But I thing we need a wholistic proposal, not just add an overload here or there.

Let's not hold this API addition hostage to all the other issues string has. So far, I'm aware of two different, but related issues:

  1. Simplified equality check

``` diff
namespace System {
public sealed class String {

  • public bool EqualsIgnoreCase(string value);
  • public static bool EqualsIgnoreCase(string a, string b);
    }
    }
    ```

    1. Contains with StringComparsion

``` diff
namespace System {
public sealed class String {

  • public bool Contains(string value, StringComparsion comparisonType);
    }
    }
    ```

@weshaggard @KrzysztofCwalina Any pushback on the proposal for EqualsIgnoreCase?

And don't forget pls about related Slice[char] operations.

Am I the only one who thinks str.EqualsIgnoreCase(...) is a bad idea?

Sounds like this opens the door for adding lots of *IgnoreCase methods for other cases. Some methods like Compare, StartsWith and EndsWith already have a pattern of using bool ignoreCase.

So, why not String.Equals(string other, bool ignoreCase)?

I'm being picky, but I'd like to see less patterns appear just for personal preferences. Much better to just follow what is already there.

@nvivo, I am not sure if it's bad or not, but I think we need to be super careful adding APIs to such basic types like string. We already have so many options here (comparers, interfaces, delegates, supporting enums, etc.). This is why I said I think we need a holistic approach, i.e. a proposal listing problems with comparison APIs in BCL, paint picture how we would like the APIs to look in the future, and an engineering plan to get us there.

@terrajobst, normally I would agree with you, but not in case of System.String. I really don't think we can add APIs to string without proper design/spec (like in the old days)

Definitely should have operator level support for OrdinalIgnoreCase, i can already write extension methods. We don't need a BCL method, we need an operator

In my case I do not actually need an operator. I would like to write simple _switch_ for every case I get. But now I end up with multi-chain if...else if... else if... else...if...etc.
I wish to be able have 1 line conversion to the simple string-like class with OrdinalIgnoreCase comparison behavior.
So I would probably make own class with compare operator override and replace all those non maintainable chains in code.

@DmitriySokhach that sounds alot like pattern matching. https://github.com/YangFan789/PatternMatchingExtension might be relevant to you

+1

@shanselman and @terrajobst What was ever the outcome of this... we are doing this a lot in our projects
string.Equals("String1","String1", StringComparison.OrdinalIgnoreCase)

was an extension ever entered in 4.6 or C# 6 feature set. Or are people still creating their own extension methods or operators?

@DmitriySokhach, you could do this:

struct OrdinalIgnoreCaseString : IEquatable<OrdinalIgnoreCaseString>, IEquatable<string> {
    string _str;
    public OrdinalIgnoreCaseString(string str) {
        _str = str;
    }

    public static implicit operator OrdinalIgnoreCaseString(string str) {
        return new OrdinalIgnoreCaseString(str);
    }

    public static explicit operator string(OrdinalIgnoreCaseString str) {
        return str._str;
    }

    public bool Equals(string other) {
        return _str.Equals(other, StringComparison.OrdinalIgnoreCase);
    }

    public bool Equals(OrdinalIgnoreCaseString other) {
        return _str.Equals(other._str, StringComparison.OrdinalIgnoreCase);
    }
}

@KrzysztofCwalina i created this on a gist since this seems like something worth saving! https://gist.github.com/dotnetchris/75f2cf6137b8e0e37961a3301e1dad5b

@dotnetchris, great; thanks!

We currently don't have a proposal to review, thus I've marked it as api-needs-work. We'll take a look whether someone on our side will do this.

Marking as up-for-grabs so that anybody can submit an API Proposal to look at.

Something that makes this a little bit less painful with C# 6:

using static System.StringComparer;
if (OrdinalIgnoreCase.Equals(alpha, beta))
{ 
    // ...
}

My personal solution would be to throw our the StringComparison enum completely, and replace all its uses with the StringComparer static instances. All methods that take StringComparison can easily be rewritten using this.

The big issue is that you always need a second to thing about which one of the two you need. The typing is not the problem, that's what intellisense/autocomplete is for

Summarising all the ideas & slipping my cent in :^)

I think it's fair to provide static versions of the methods as well since we have them for all the other overloads/methods.

1. Providing an overload of Equals(string) that takes an additional boolean parameter ignoreCase

Suggested API addition

public sealed class String
{
    public string Equals(string str, bool ignoreCase);
    public static string Equals(string a, string b, bool ignoreCase);
}

Pros

  • Matches what other methods do, such as IndexOf, Contains, EndsWith, Compare etc

    Cons

  • Overloads of other methods that accept bool does not indicate culture insensitive comparison, but only case sensitivity... though, it might be fine in this instance since string.Equals is ordinal by default.

    Usage

if (str1.Equals(str2, true))
{
    // The two strings are considered equal, disregarding culture/case
}

2. Providing a new method EqualsIgnoreCase(string)

Suggested API addition

public sealed class String
{
    public string EqualsIgnoreCase(string value);
    public static string EqualsIgnoreCase(string a, string b);
}

Pros

  • Maybe... shorter to type?

    Cons

  • Same as the disadvantage described in dotnet/corefx#1; implied culture insensitivity.

  • Missing the matching culture sensitive alternative, What if I want to do culture sensitive / case insensitive comparison?

Usage

if (str1.EqualsIgnoreCase(str2))
{
    // ...
}

3. Providing a new method EqualsOrdinalIgnoreCase(string)

Suggested API addition

public sealed class String
{
    public string EqualsOrdinalIgnoreCase(string value);
    public static string EqualsOrdinalIgnoreCase(string a, string b);
}

Pros

  • Being explicit about both the culture/case insensitivity

    Cons

  • About as long to type

    Usage

if (str1.EqualsOrdinalIgnoreCase(str2))
{
// ...
}

4. Providing a new method EqualsOrdinal that takes a string, and an overload that takes an additional parameter ignoreCase. Also an additional Equals(string, bool) overload

Suggested API addition

public sealed class String
{
    public string Equals(string value, bool ignoreCase);
    public static string Equals(string a, string b, bool ignoreCase);
    public string EqualsOrdinal(string value);
    public string EqualsOrdinal(string value, bool ignoreCase);
    public static string EqualsOrdinal(string a, string b);
    public static string EqualsOrdinal(string a, string b, bool ignoreCase);
}

(Maybe ignoreCase can be optional?)

Pros

+ Matches what other methods do, by providing a bool ignoreCase parameter ~~
~~+ Matches how Compare has a separate method group dedicated for culture insensitive comparison (Compare and CompareOrdinal, although CompareOrdinal has no instance methods)

+ Explicit about culture insensitivity, which solves the disadvantage described above
+ Also provides easy culture insensitive/case sensitive comparison

Cons

- To be discussed

Usage

if (str1.EqualsOrdinal(str2, true))
{
    // The two strings are considered equal, disregarding culture/case
}

Apparently I was thinking that string.Equals(string) defaults to current culture when its not. 😅

Existing alternatives

1. using the current string.Equals(string, StringComparison)

if (str1.Equals(str2, StringComparison.OrdinalIgnoreCase))
{
    // ...
}

2. Using StringComparer (Suggested by @\andersstorhaug)

using static System.StringComparer;
if (OrdinalIgnoreCase.Equals(alpha, beta))
{ 
    // ...
}

Adding EqualsIgnoreCase() will add more complicity and open a way for bugs I believe.
In PowerShell repo we came to always use explicit values for string comparisons. PowerShell is almost always case-insensitive but not always. It uses currentculture for console output but invariant culture or sometimes ordinal in engine. Therefore, we must be careful and always use explicit values so that a code read will always see design intentions. I think it is true for every considerable application.

This issue is going on six years old and has not had any activity in over a year. We're clearly not going to change the behavior of the existing string.Equals or string.operator== APIs.

The proposal that got the most upvotes in this issue was introducing an EqualsIgnoreCase API, but even that spawned a bit of disagreement.

There are allusions to more fundamental problems with the string APIs and equality checking, but as far as I can tell nobody in this thread has articulated them. __If somebody wants to take a stab at clearly explaining the problem, that would help us properly address this issue.__

Is the problem that we'd prefer the default equality check be ordinal case-insensitive instead of ordinal case-sensitive? (This isn't going to happen.) Is the problem that other string APIs are culture-aware by default? (That's not really relevant to an accelerator for EqualsIgnoreCase.) Is the problem that this is just too much typing and there needs to be an accelerator? (We could address that at an API level or at a language level.)

As an example of a language accelerator that requires no API additions, there's always:

using static System.StringComparison;

string candidate = ...;
if (candidate.Equals("hello", OrdinalIgnoreCase)) { /* do something */ }

If the goal is to avoid writing the literal text StringComparison more than once per file, does this solve the issue? Without a clearly articulated problem statement, this issue is not currently actionable and is likely to be closed.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

omajid picture omajid  Â·  3Comments

matty-hall picture matty-hall  Â·  3Comments

omariom picture omariom  Â·  3Comments

iCodeWebApps picture iCodeWebApps  Â·  3Comments

Timovzl picture Timovzl  Â·  3Comments