RobThree / NGeoNames

Inspired by https://github.com/AReallyGoodName/OfflineReverseGeocode
MIT License
87 stars 20 forks source link

ReverseGeoCodeSearch.NearestNeighbourSearch (Culture Bug) #6

Closed justlearntutors closed 6 years ago

justlearntutors commented 6 years ago

If the culture is different than "en", the returned cities are not related to the provided values.

Code sample.

System.Globalization.CultureInfo.DefaultThreadCurrentCulture = System.Globalization.CultureInfo.GetCultureInfo("da");
System.Globalization.CultureInfo.DefaultThreadCurrentUICulture = System.Globalization.CultureInfo.GetCultureInfo("da");
var CitiesNearbyStart = ReverseGeoCodeSearch.CreateFromLatLong(double.Parse(latitude), double.Parse(longitude));
var CitiesNearby = ReverseGeoCodeSearch.NearestNeighbourSearch(CitiesNearbyStart, 50);
var CitiesListNearby = CitiesNearby.ToList();

Working code sample

System.Globalization.CultureInfo.DefaultThreadCurrentCulture = System.Globalization.CultureInfo.GetCultureInfo("en");
System.Globalization.CultureInfo.DefaultThreadCurrentUICulture = System.Globalization.CultureInfo.GetCultureInfo("en");
var CitiesNearbyStart = ReverseGeoCodeSearch.CreateFromLatLong(double.Parse(latitude), double.Parse(longitude));
var CitiesNearby = ReverseGeoCodeSearch.NearestNeighbourSearch(CitiesNearbyStart, 50);
var CitiesListNearby = CitiesNearby.ToList();
RobThree commented 6 years ago

Did you debug this?

What do double.Parse(latitude) and double.Parse(longitude) return in both cases? Because I'm pretty sure (99%) that that's where your problem is. This is most likely not an NGeoNames bug. double.Parse(...) is culture-aware and, thus, will have different outcomes for when parsing a value 1.234 or 1,234.

Please, next time provide a clear example, including how you debugged the issue, what the results are/were and what the expected results are/were.

justlearntutors commented 6 years ago

latitude = 57.048 longitude = 9.9187

The returned city names were not from Denmark. It did not work for any latitude and longitude, when the culture was "da".

RobThree commented 6 years ago

Please provide the exact values for latitude and longitude (both should be strings) and please provide the exact outcomes of the double.Parse(latitude) and double.Parse(longitude) in both cultures.

latitude = 57.048 longitude = 9.9187

Those are parsed values, those are not the string values you're providing to double.Parse().

justlearntutors commented 6 years ago

Culture is "en" [0] Name: Aalborg CountryCode: DK

Culture is "da" [0] Name: Tsafe CountryCode: NG

justlearntutors commented 6 years ago

Maybe it is something to do with comma and dot.

We use comma for numbers in Denmark and not dot.

RobThree commented 6 years ago

Try this:

using System;
using System.Globalization;

public class Program
{
    public static void Main()
    {
        var latitude = "57.048";
        var longitude = "9.9187";

        Test("en", latitude, longitude);
        Test("da", latitude, longitude);
    }

    private static void Test(string culture, string lat, string lon)
    {
        SetCulture(culture);
        Console.WriteLine("Cult: " + culture + ", Lat: " + double.Parse(lat) + ", Lon: " + double.Parse(lon));
    }

    private static void SetCulture(string culture)
    {
        var ci = CultureInfo.GetCultureInfo(culture);

        CultureInfo.DefaultThreadCurrentCulture = ci;
        CultureInfo.DefaultThreadCurrentUICulture = ci;
    }

}

Output:

Cult: en, Lat: 57.048, Lon: 9.9187
Cult: da, Lat: 57048, Lon: 99187

This is because culture "da" expects a comma as decimal-separator.

RobThree commented 6 years ago

Maybe it is something to do with comma and dot.

Exactly my point and what I said here. Your parsing is wrong; this has nothing to do with NGeoNames (and is why you should debug and provide a clear testcase before creating an issue and waste someones time).

We use comma for numbers in Denmark and not dot.

So you'll need to rethink your switching of culture 😉 You can provide an extra argument to double.Parse() which will make it culture-insensitive:

double.Parse(myvalue, CultureInfo.InvariantCulture)

So in the above example change:

Console.WriteLine("Cult: " + culture + ", Lat: " + double.Parse(lat) + ", Lon: " + double.Parse(lon));

To:

Console.WriteLine("Cult: " + culture + ", Lat: " + double.Parse(lat, CultureInfo.InvariantCulture) + ", Lon: " + double.Parse(lon, CultureInfo.InvariantCulture));

...and you're good.

If I were you I'd put this into it's own method:

private static double MyParseDouble(string value)
{
    return double.Parse(value, CultureInfo.InvariantCulture);
}

...so you can change the previously referred line to:

Console.WriteLine("Cult: " + culture + ", Lat: " + MyParseDouble(lat) + ", Lon: " + MyParseDouble(lon));