Skip to content

Remove lat lon bound checks from GeoLocation #4225

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

Merged
merged 1 commit into from
Nov 15, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 12 additions & 28 deletions src/Nest/QueryDsl/Geo/GeoLocation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,8 @@ public class GeoLocation : IEquatable<GeoLocation>, IFormattable
/// <summary>
/// Represents a Latitude/Longitude as a 2 dimensional point.
/// </summary>
/// <param name="latitude">Value between -90 and 90</param>
/// <param name="longitude">Value between -180 and 180</param>
/// <exception cref="ArgumentOutOfRangeException">If <paramref name="latitude" /> or <paramref name="longitude" /> are invalid</exception>
public GeoLocation(double latitude, double longitude)
{
if (!IsValidLatitude(latitude))
throw new ArgumentOutOfRangeException(string.Format(CultureInfo.InvariantCulture,
"Invalid latitude '{0}'. Valid values are between -90 and 90", latitude));
if (!IsValidLongitude(longitude))
throw new ArgumentOutOfRangeException(string.Format(CultureInfo.InvariantCulture,
"Invalid longitude '{0}'. Valid values are between -180 and 180", longitude));

Latitude = latitude;
Longitude = longitude;
}
Expand Down Expand Up @@ -62,35 +52,27 @@ public bool Equals(GeoLocation other)
public string ToString(string format, IFormatProvider formatProvider) => ToString();

/// <summary>
/// True if <paramref name="latitude" /> is a valid latitude. Otherwise false.
/// Checks if <paramref name="latitude" /> is a valid latitude between -90 and 90, inclusive.
/// </summary>
/// <param name="latitude"></param>
/// <returns></returns>
public static bool IsValidLatitude(double latitude) => latitude >= -90 && latitude <= 90;

/// <summary>
/// True if <paramref name="longitude" /> is a valid longitude. Otherwise false.
/// Checks if <paramref name="longitude" /> is a valid longitude between -180 and 180, inclusive.
/// </summary>
/// <param name="longitude"></param>
/// <returns></returns>
public static bool IsValidLongitude(double longitude) => longitude >= -180 && longitude <= 180;

/// <summary>
/// Try to create a <see cref="GeoLocation" />.
/// Return
/// <value>null</value>
/// if either <paramref name="latitude" /> or <paramref name="longitude" /> are invalid.
/// </summary>
/// <param name="latitude">Value between -90 and 90</param>
/// <param name="longitude">Value between -180 and 180</param>
/// <param name="latitude"></param>
/// <param name="longitude"></param>
/// <returns></returns>
public static GeoLocation TryCreate(double latitude, double longitude)
{
if (IsValidLatitude(latitude) && IsValidLongitude(longitude))
return new GeoLocation(latitude, longitude);

return null;
}
// TODO: Remove in 8.0 as without bounds checks, provides no value
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++

public static GeoLocation TryCreate(double latitude, double longitude) => new GeoLocation(latitude, longitude);

public override string ToString() =>
Latitude.ToString("#0.0#######", CultureInfo.InvariantCulture) + "," +
Expand Down Expand Up @@ -157,6 +139,8 @@ public GeoCoordinate(double latitude, double longitude, double z) : base(latitud
/// <summary>
/// Creates a new instance of <see cref="GeoCoordinate" /> from an array
/// of 2 or 3 doubles, in the order Latitude, Longitude, and optional Z value.
/// Returns null if coordinates are null
/// <exception cref="ArgumentOutOfRangeException">If the array does not contain 2 or 3 values</exception>
/// </summary>
public static implicit operator GeoCoordinate(double[] coordinates)
{
Expand All @@ -168,11 +152,11 @@ public static implicit operator GeoCoordinate(double[] coordinates)
return new GeoCoordinate(coordinates[0], coordinates[1]);
case 3:
return new GeoCoordinate(coordinates[0], coordinates[1], coordinates[2]);
default:
throw new ArgumentOutOfRangeException(
nameof(coordinates),
$"Cannot create a {nameof(GeoCoordinate)} from an array that does not contain 2 or 3 values");
}

throw new ArgumentOutOfRangeException(
nameof(coordinates),
$"Cannot create a {nameof(GeoCoordinate)} from an array that does not contain 2 or 3 values");
}
}
}