Reputation: 25359

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

I am working with a geo-coding API and need to represent the coordinates 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!

Upvotes: 40

Answers (8)

Florian Greinacher

Reputation: 14784

No need for an interface in your case. Just make it a plain old struct. This will prevent any unnecessary boxing when passing the struct via its interface.

Upvotes: 1

Irwin

Reputation: 12829

I really like the justification and guidance provided by this coordinate library on codeplex In it, they use classes and to represent the actual values for lat and long they use float.

Upvotes: 1

Jon Skeet

Reputation: 1503070

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.

Upvotes: 60

sehe

Reputation: 393664

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

Upvotes: 9

supercat

Reputation: 81247

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:

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.
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.

Upvotes: 4

Sharath Satish

Reputation: 104

Personally, I prefer to use classes even if they are simple ones like your LatLong. I haven't used structs since my c++ days. The additional advantage of classes are the ability in the future to extend them if more complex functionality is required.

I do agree with the other folks on this thread that an Interface seems like an overkill though since I don't have full context of what you are using this object for it may be required in your application.

Lastly, your GetHashCode seems to be a glorified way of doing "Lat * Long". I am not sure if that is a safe bet. Also, if you plan to use GetHashCode many times in your application I would recommend keeping it simple to improve the performance of the method.

Upvotes: -1