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

Add String.Contains() and StringComparison enum #9

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Gaulomatic
Copy link

Hi,

I have added a static String.Contains() method with an optional parameter of a also added StringComparison enum. Makes string comparisons much more C#-like.

@sevensc
Copy link
Owner

sevensc commented Aug 8, 2018

Good idea, but i would rather create a prototype for strings.
usage something like this.

'this'.Contains('hi');

implementation something like this.

String.prototype.Contains = function (value: string): boolean {
    return this.indexOf(value) !== -1;
}

@Gaulomatic
Copy link
Author

Gaulomatic commented Aug 8, 2018

I thought about this as well and although (from an object orientated perspective) this makes absolute sense, I decided against it. The RxJS team also moved away from prototype manipulation. I watched a talk about this topic in the aftermath of the 6.0 release. I can't quite remember why they went this route, but from what I can tell, it can be a bit complicated to make sure to get the 'new' prototype and get IntelliSense for TypeScript. I extended the error object for an app and declared an interface. But you need to make sure, that the prototype is indeed modified, because the TS compiler won't complain (because he just don't really know).

EDIT:
To make clear what I am talking about: The TS compiler in WebStorm and VSCode is by now already confused when using your String class, it messes up the imports all the time, because there is a String whatever somewhere under the hood.

C# has for the most part static and instance implementations as well, so you do not need to check for null beforehand. This makes the using code look cleaner. So maybe we could use both as well? It's just a preference thing at the end of the day.

@sevensc
Copy link
Owner

sevensc commented Dec 6, 2019

Could you do a test for the Contains Method?

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

Successfully merging this pull request may close these issues.

None yet

2 participants