Should I use a struct or a class to represent a Lat/Lng coordinate?

Question

I am working a with a geo-coding API and need to represent the coordinate of a returned point as a Latitude / Longitude pair. However, I am unsure whether to use a struct or a class for this. My initial thought was to use a struct, but they seem to be generally frowned upon in C# (for instance, Jon Skeet mentions in this answer that, "I almost never define custom structs"). Performance and memory usage are not critical factors in the application.

So far I have come up with these two implementations based on a simple interface:

Interface

public interface ILatLng
{
    double Lat { get; }
    double Lng { get; }
}

LatLng Class Implementation

public class CLatLng : ILatLng
{
    public double Lat { get; private set; }
    public double Lng { get; private set; }

    public CLatLng(double lat, double lng)
    {
        this.Lat = lat;
        this.Lng = lng;
    }

    public override string ToString()
    {
        return String.Format("{0},{1}", this.Lat, this.Lng);
    }

    public override bool Equals(Object obj)
    {
        if (obj == null)
            return false;

        CLatLng latlng = obj as CLatLng;
        if ((Object)latlng == null)
            return false;

        return (this.Lat == latlng.Lat) && (this.Lng == latlng.Lng);
    }

    public bool Equals(CLatLng latlng)
    {
        if ((object)latlng == null)
            return false;

        return (this.Lat == latlng.Lat) && (this.Lng == latlng.Lng);
    }


    public override int GetHashCode()
    {
        return (int)Math.Sqrt(Math.Pow(this.Lat, 2) * Math.Pow(this.Lng, 2));
    }
}

LatLng Struct Implementation

public struct SLatLng : ILatLng
{
    private double _lat;
    private double _lng;

    public double Lat
    {
        get { return _lat; }
        set { _lat = value; }
    }

    public double Lng
    {
        get { return _lng; }
        set { _lng = value; }
    }

    public SLatLng(double lat, double lng)
    {
        this._lat = lat;
        this._lng = lng;
    }

    public override string ToString()
    {
        return String.Format("{0},{1}", this.Lat, this.Lng);
    }
}

Performing some tests I've come to the following findings:

• A struct always has a parameterless constructor, which means you can't force it to be instantiated with a constructor which expects two properties (for lat and lng), as you can with a class.

• A struct (being a value type) can never be null, so will always contain a value. But you can still do stuff like this if implementing an interface:

  ILatLng s = new SLatLng(); s = null;

So does it make sense for a struct to use an interface in this case?

• If I use a struct do I need to override Equals, GetHashCode() etc. ? My tests indicate comparisons work correctly without doing so (unlike with a class) - so is it necessary?

• I feel more 'comfortable' using classes, so is it best to just stick with them as I'm more aware of how they behave? Will people using my code be confused by value-type semantics, especially when working to an interface?

• In the CLatLng implementation, does the override of GetHashCode() seem OK? I 'stole' it from this article, so am unsure!

Any help or advice gratefully received!

Solution 1

I can't see any point in having an interface for this, to be honest.

I would just create a struct, but make it immutable - mutable structs are a really bad idea. I'd also use full Latitude and Longitude as the property names. Something like this:

public struct GeoCoordinate
{
    private readonly double latitude;
    private readonly double longitude;

    public double Latitude { get { return latitude; } }
    public double Longitude { get { return longitude; } }

    public GeoCoordinate(double latitude, double longitude)
    {
        this.latitude = latitude;
        this.longitude = longitude;
    }

    public override string ToString()
    {
        return string.Format("{0},{1}", Latitude, Longitude);
    }
}

I'd then also implement IEquatable<GeoCoordinate> and override Equals and GetHashCode, e.g.

public override bool Equals(Object other)
{
    return other is GeoCoordinate && Equals((GeoCoordinate) other);
}

public bool Equals(GeoCoordinate other)
{
    return Latitude == other.Latitude && Longitude == other.Longitude;
}

public override int GetHashCode()
{
    return Latitude.GetHashCode() ^ Longitude.GetHashCode();
}

Note that you need to be aware of the normal dangers of performing equality comparisons on doubles - there's not much alternative here, but two values which look like they should be equal may not be...

The point about the parameterless constructor is a reasonable one, but I suspect you'll find it won't actually bite you.

Solution 2

Make it a struct, for performance.

• The performance benefit will multiply manytimes when you handle e.g. arrays of these structs. Note that e.g. System.Collections.Generic.List correctly handles unboxed storage of the element type in .Net Arrays, so it applies to generic containers just as well.

• Note that the fact that you can't have a constructor is completely negated by the C# 3.5+ intializer syntax:

  new SLatLng { Lat = 1.0, Lng = 2.0 }
  

Cost of interface usage

Note that adding the interface inevitably reduces performance: interfaces cannot define fields, a struct without fields is hardly useful. That leaves only one realistic scenario: the interface requires you to define the properies to access fields.

If you are obliged to use the properties (via getter/setter) you will loose performance of direct access. Compare:

With interface

public class X
{
    interface ITest { int x {get; } }
    struct Test : ITest
    {
        public int x { get; set; }
    }

    public static void Main(string[] ss)
    {
        var t = new Test { x=42 };
        ITest itf = t;
    }
}

Generate setter invocation and boxing

.method public static  hidebysig 
       default void Main (string[] ss)  cil managed 
{
    // Method begins at RVA 0x20f4
.entrypoint
// Code size 29 (0x1d)
.maxstack 4
.locals init (
    valuetype X/Test    V_0,
    class X/ITest   V_1,
    valuetype X/Test    V_2)
IL_0000:  ldloca.s 0
IL_0002:  initobj X/Test
IL_0008:  ldloc.0 
IL_0009:  stloc.2 
IL_000a:  ldloca.s 2
IL_000c:  ldc.i4.s 0x2a
IL_000e:  call instance void valuetype X/Test::set_x(int32)
IL_0013:  ldloc.2 
IL_0014:  stloc.0 
IL_0015:  ldloc.0 
IL_0016:  box X/Test
IL_001b:  stloc.1 
IL_001c:  ret 
} // end of method X::Main

Without interface

public class Y
{
    struct Test
    {
        public int x;
    }

    public static void Main(string[] ss)
    {
        var t = new Test { x=42 };
        Test copy = t;
    }
}

Generates direct assignment and (obviously) no boxing

// method line 2
.method public static  hidebysig 
       default void Main (string[] ss)  cil managed 
{
    // Method begins at RVA 0x20f4
.entrypoint
// Code size 24 (0x18)
.maxstack 2
.locals init (
    valuetype Y/Test    V_0,
    valuetype Y/Test    V_1,
    valuetype Y/Test    V_2)
IL_0000:  ldloca.s 0
IL_0002:  initobj Y/Test
IL_0008:  ldloc.0 
IL_0009:  stloc.2 
IL_000a:  ldloca.s 2
IL_000c:  ldc.i4.s 0x2a
IL_000e:  stfld int32 Y/Test::x
IL_0013:  ldloc.2 
IL_0014:  stloc.0 
IL_0015:  ldloc.0 
IL_0016:  stloc.1 
IL_0017:  ret 
} // end of method Y::Main

Solution 3

Structs and value types are entities outside the .net Object hierarchy, but every time you define a struct the system also defines a pseudo-class derived from ValueType which largely behaves like the struct; widening conversion operators are defined between the struct and the pseudo-class. Note that variables, parameters, and fields which are declared to be of interface type are always processed as class objects. If something is going to be used as an interface to any significant degree, in many cases it may as well be a class.

Although some people rail about the evils of mutable structs, there are many places where mutable structs with value semantics would be very useful were it not for deficiencies in the system's handling of them. For example:

1. Methods and properties which mutate "self" should be tagged with an attribute that would forbid their application in read-only contexts; methods without such attribute should be forbidden from mutating "self" unless compiled with a compatibility switch.
2. There should be means of passing references to structs or fields thereof by turning certain expressions inside out, or by having a standard type of property-delegate-pair, so as to facilitate things like myDictOfPoints("George").X++;

Often, value type sematics would far more "expected" than reference semantics, were it not only for the fact that the former are so poorly supported in some common languages.

PS--I would suggest that while mutable structures are often a good and appropriate thing, structure members that mutate "self" are handled badly and should be avoided. Either use functions which will return a new structure (e.g. "AfterMoving(Double distanceMiles, Double headingDegrees)") which would return a new LatLong whose position was where one would be after moving a specified distance), or use static methods (e.g. "MoveDistance(ref LatLong position, Double distanceMiles, Double headingDegrees)"). Mutable structs should generally be used in places where they essentially represent a group of variables.

Solution 4

I would use a struct. A class is way overkill for the simple use here- you can look at other structs like Point for an example.

Solution 5

You're trying to make a mutable struct, that's a no-no. Especially since it implements an interface!

If you want your LatLng type to be mutable, stick with reference type. Otherwise struct is fine for this particular example.

View more solutions

Comments

[Dan Diplo]

Thanks, that makes sense. Is the way I have overriden Equals and GetHashCode in the CLatLng class OK to use in a struct?

[Jon Skeet]

@Dan: No, you'd want to take into account the fact that it can't be null, as it's a value type. Will add some code...

[sehe]

The additional advantage of classes are the ability in the future to extend them if more complex functionality is required. - You must be a java programmer :) Struct can easily be extended. Also, nothing prevents you from writing extensions to valuetypes, write (generic) decorator classes for valuetypes, etc. etc. If you want 'is-a' semantics, you can even emulate that by using an interface and adding it to your struct. In short: YAGNI

[Dan Diplo]

That's interesting. Whilst performance isn't really a major factor, I can see that there is no reason not to consider it when everything else is equal. You also make a good point regarding the C# 3.5+ intializer syntax and constructor. Thanks for your input.

[Dan Diplo]

I was also uneasy with the GetHashCode method, but was unsure what else to do. Jon has a good suggestion I'm going to crib!

[supercat]

@Dan Diplo: You're welcome. In older languages, variables and fields almost always used value-type semantics (parameters and constants were a mixed bag). If one wanted to, e.g. increment an integer pointed to by a variable, one would have to use a dereferencing operator. A later philosophy seems to have emerged that member accesses should dereference by default. Personally, I wish different notation was used for direct structure member access and indirect class member access; it would have avoided a lot of confusion.

[supercat]

FYI, one possible reason for using an interface might be to allow for the possibility of different coordinate systems. One could have a method which given e.g. an ILatLng, would return an IGridCoordinate by checking whether the passed-in instance implemented the latter type and, if not, creating an object which would wrap the original one and implement both interfaces, doing numerical conversion as needed. Repeatedly conversions between LatLong and GridCoordinate struct types could cause progressive rounding errors, but interface-based conversions, done properly, would not.

[heltonbiker]

For the (late) sake of completeness, the Open Geospatial Consortium (OGC/ISO) set of standards suggests to represent a 2D coordinate position as an ordinate pair (double[2]), with latitude coming always first. Not that this matters, but since there is a standard for that... ;o)

[heltonbiker]

From the ISO 19111 standard (Spatial Referencing by Coordinates, section 6.1): "In this International Standard, a coordinate is one of n scalar values that define the position of a single point. A coordinate tuple is an ordered list of coordinates. The coordinate tuple is composed of one, two or three spatial coordinates. The coordinates shall be mutually independent and their number shall be equal to the dimension of the coordinate space. The order of the elements of the tuple and their unit of measure are parts of the coordinate reference system definition."

[Wouter van Nifterick]

Using Lat * Lon as a hash is not a safe bet at all.. Coordinate (52,4) will have the same hash as (4,52).

[almcnicoll]

@JonSkeet: I'd love to know why mutable structs are a bad idea. I'm not questioning your statement - just interested in why. Perhaps an external link would enhance this answer?

[Jon Skeet]

@almcnicoll: See stackoverflow.com/questions/441309/why-are-mutable-structs-e‌​vil to start with.

[Amadeus Sánchez]

@JonSkeet. Is there is a reason why you used doubles to represent the latitude and longitude rather than floats? Thanks.

[Jon Skeet]

@AmadeusSánchez: Well to start with, that's what the OP has.

[Nick N.]

@JonSkeet what about this new class: msdn.microsoft.com/en-us/library/… ? Would you still prefer a custom object because it is cleaner?

[Jon Skeet]

@NickN.: If I were in an environment where that class was available, I might use it - but not necessarily (given that it includes other information which I know I wouldn't have).