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

Option Compare Ordinal (and OrdinalIgnoreCase?) #193

Closed
AnthonyDGreen opened this issue Oct 27, 2017 · 3 comments
Closed

Option Compare Ordinal (and OrdinalIgnoreCase?) #193

AnthonyDGreen opened this issue Oct 27, 2017 · 3 comments
Assignees

Comments

@AnthonyDGreen
Copy link
Contributor

Two problems:

  1. VB string comparison is not exactly the same as String.Equals; specifically, VB treats empty strings and null strings as equal. If neither string is null, and if Option Compare Text is in effect, VB uses CurrentCultureIgnoreCase. Otherwise (Option Compare Binary), VB uses Ordinal string comparison.

Due to this special rule in VB that treats null and empty as equivalent, VB string comparison calls out to a runtime method (Operators.CompareString). This method appears in the expression trees we produce from LINQ and every LINQ provider ever blows up on it. This adds an extra burden to supporting VB in LINQ providers and authors very often have no idea what to do with it.

Beyond that, I think it's weird to educate new programmers that empty and null strings are equal as in most of the .NET ecosystem that will not be the case.

  1. If you want case-insensitive string comparisons you're left with some options.
    a) Abandon such lovely built-in constructs like the Select Case statement.
    b) ToUpper() all of your strings before comparing them.
    c) Turn Option Compare Text.

(a) is just terrible. VB is a beautifully expressive language and that expressiveness shouldn't need to be jettisoned in order to do a very common thing--case insensitive string comparison. This could be felt even more if we add other string related constructs like #140 or an In operator.

(b) Is against .NET performance best-practice because of the extra string allocations. Potentially massive waste if the strings are large.

(c) Still uses VB special rules about null and empty strings and also uses the current UI culture rather than the invariant culture or a case-insensitive ordinal (binary) comparison.

If we extend Option Compare to support more options and we implement #117 we could restore some of VB's productiveness with strings, extend the utility of built-in language constructs, and not force users into wasteful habits.

Question
If there a way to get the same implicit-argument behavior we do today with Option Compare such that APIs which take a ComparisonType or an EqualityComparer(Of String) automatically take the correct inputs based on the current setting (e.g. VBs Distinct operator given a sequence of strings or the Dictionary(Of String, TValue) constructor)? Seems unlikely since every method that takes such an argument has an overload that doesn't take it. Could that be done performantly? Should we do that? Maybe an analyzer that forces users to be consistent and explicit would be better.

@reduckted
Copy link
Contributor

This doesn't directly answer your question, but I would dearly love an Option Compare value that results in the code using the = operator from a string instead of calling off to some helper method. Being able to use strings like normal .NET strings would be super-awesome, and would mean you don't have to jump through hoops to get LINQ providers to work.

As an example, (and if my memory serves me correctly), the IVSExtensionRepository implementation from Visual Studio's extensibility model only supports the = operator, so you cannot use it in VB at all because you can't write code like this:

 _repository.CreateQuery<GalleryEntry>()
    .Where(e => e.VsixID == model.ProductId)
    .AsEnumerable()
    .FirstOrDefault();

(the e.VsixID == model.ProductId is a string comparison).

For the Option Compare values, how about something like this:

Value Implementation
Text Does whatever it does today
Binary Does whatever it does today
Ordinal String.op_Equality
OrdinalIgnoreCase StringComparer.OrdinalIgnoreCase
InvariantCulture StringComparer.InvariantCulture
InvariantCultureIgnoreCase StringComparer.InvariantCultureIgnoreCase
CurrentCulture StringComparer.CurrentCulture
CurrentCultureIgnoreCase StringComparer.CurrentCultureIgnoreCase

Effectively the values are the two legacy values and one for each of the StringComparison enum values that map to the corresponding StringComparer (except for Option Compare Ordinal which uses the String's = operator because that's basically what the StringComparer.Ordinal value uses).

To answer your question (assuming I'm interpreting it correctly), no we should not automatically provide a string comparer to things like Dictionary(Of String, TValue) when one is not specified (that would break pretty much all existing code).

What about some sort of comparer instance that the compiler replaces with the current setting. For example:

Dim x = New Dictionary(Of String, Integer)(Microsoft.VisualBasic.CurrentStringComparer)

The compiler could translate the use of that comparer instance into the appropriate StringComparer. That would require new classes in the Microsoft.VisualBasic assembly, and that doesn't appear to have changed since v10, so I'm not sure if that's desirable.

@AnthonyDGreen
Copy link
Contributor Author

AnthonyDGreen commented Dec 7, 2017

I'm withdrawing this proposal in favor of #218.

@zspitz
Copy link

zspitz commented Nov 16, 2018

#218 only handles the expression tree rewriting. What about case-insensitive Select Case?

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

No branches or pull requests

3 participants