MicahWW / Money-Tracking-Functions

The backend to a service to help track how one's money is being spent
0 stars 0 forks source link

Replace toLower() with proper Unicode sanitation methods #10

Closed codyswanner closed 4 months ago

codyswanner commented 5 months ago

The toLower() method works fine for basic string sanitation for comparisons, but has some edge case issues. My experience with this is more in Python (which has native methods for this kind of sanitation/checking) but I know the issues are with Unicode itself.

@MicahWW I'll try to find some official sources for you to look through on this to decide if it's relevant for this project.

MicahWW commented 4 months ago

Microsoft talks about this here. In short, there is an issue with using functions that do change the casing of a letter as some languages use the English Unicode characters but they might they represent something different so trying to change the casing can change the letter completely.

To avoid this issue I think it is okay to vary slightly from using one of the 2 recommended methods below:

if (string.Compare(stringLeft, stringRight, false, CultureInfo.InvariantCulture) == 0) {
    DoSomething();
}
if (string.Compare(stringLeft, stringRight, false, CultureInfo.CurrentCulture) == 0) {
    DoSomething();
}

To ignore caseing altogether.

if (string.Compare(stringLeft, stringRight, true) == 0) {
    DoSomething();
}

I made a change and attached the branch to this issue for you to see @codyswanner, that was the only place where changing the casing happens. The only other type of string function I could find was string.IsNullOrEmpty

MicahWW commented 4 months ago

We talked a little not on this thread and you didn't bring up any concerns so I am going to merge the branch in and if you have questions/concerns go ahead and reopen the issue.

MicahWW commented 4 months ago

Turns out I made the mistake of committing the change to the main branch so the change is already moved over via 504d2c7