41

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!

Community
  • 1
  • 1
Dan Diplo
  • 25,076
  • 4
  • 67
  • 89
  • BTW, have you tested performance replacing the properties with fields (using explicit interface implementation, if you still want the interface). If the semantics of the struct are such that it represents a set of values that may be set independently, fields are semantically better than properties and will likely offer better performance. – supercat Dec 07 '12 at 19:07

8 Answers8

60

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.

Mrchief
  • 75,126
  • 20
  • 142
  • 189
Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • Thanks, that makes sense. Is the way I have overriden `Equals` and `GetHashCode` in the `CLatLng` class OK to use in a struct? – Dan Diplo May 27 '11 at 12:44
  • 1
    @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... – Jon Skeet May 27 '11 at 12:47
  • 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. – supercat Aug 17 '12 at 16:32
  • 1
    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 Apr 04 '13 at 13:13
  • 1
    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." – heltonbiker Apr 04 '13 at 13:15
  • @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? – almcnicoll Jul 31 '15 at 19:08
  • 2
    @almcnicoll: See http://stackoverflow.com/questions/441309/why-are-mutable-structs-evil to start with. – Jon Skeet Jul 31 '15 at 19:29
  • @JonSkeet. Is there is a reason why you used doubles to represent the latitude and longitude rather than floats? Thanks. – Amadeus Sanchez Dec 16 '15 at 19:57
  • @AmadeusSánchez: Well to start with, that's what the OP has. – Jon Skeet Dec 16 '15 at 20:38
  • @JonSkeet what about this new class: https://msdn.microsoft.com/en-us/library/system.device.location.geocoordinate(v=vs.110).aspx ? Would you still prefer a custom object because it is cleaner? – Nick N. Feb 25 '16 at 10:00
  • 2
    @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). – Jon Skeet Feb 25 '16 at 10:01
9

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
sehe
  • 374,641
  • 47
  • 450
  • 633
  • 1
    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 May 27 '11 at 12:58
4

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.

supercat
  • 77,689
  • 9
  • 166
  • 211
  • @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 Jun 16 '11 at 14:50
3

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.

Puppy
  • 144,682
  • 38
  • 256
  • 465
3

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.

Dyppl
  • 12,161
  • 9
  • 47
  • 68
2

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

Florian Greinacher
  • 14,478
  • 1
  • 35
  • 53
1

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.

Irwin
  • 12,551
  • 11
  • 67
  • 97
0

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.

  • 2
    `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 – sehe May 27 '11 at 12:54
  • 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! – Dan Diplo May 27 '11 at 12:59
  • 1
    Using `Lat * Lon` as a hash is not a safe bet at all.. Coordinate (52,4) will have the same hash as (4,52). – Wouter van Nifterick Jun 18 '13 at 18:20