Improve code quality (#88)

* replace GeoJsonObject.Type by abstract read-only property

The Type now resolves to a constant enum value for each subclass. Benefits:
- allows to remove setter -> objects are more immutable now
- reduces memory footprint of each object by one string
- impossible to accidentally forget to set the Type in a subclass constructor

* make Point.Coordinates readonly & remove parameterless constructor

* Increase immutability of Polygon and LineString & avoid explicit List types

Both classes' coordinates field is read-only now and of IReadOnlyList<T> type instead of List<T>, which prevents manipulation after creation.
For this to work, additional constructors were introduced that take nested double enumerables as in the GeoJSON spec.

* Rename Converters & extend previous work to more classes

The previous converter names were highly misleading because e.g. PolygonConverter did NOT read/write Polygon object but instead only the Coordinates property of a Polygon.

* Make MultiPoint & MultiPolygon immutable and improve corresponding converters

* Make Position formally immutable (and smaller in memory)

* Various small code style and documentation improvements

* Let Feature<TGeometry> derive from Feature<TGeometry, TProps>

Avoids duplication of code.

* Streamline the style of JSON converters

* Remove unused (and, from its XML doc, probably very old) ParsingException class

* Position.cs: remove redundant null checks & remove misleading comment

* Add missing and fix existing XML docs.

* Fix for net35 target: replace IReadOnlyList by ReadOnlyCollection

Author: supermihi

src/GeoJSON.Net.Tests/Geometry/LineStringTests.cs

@@ -93,7 +93,7 @@ public void Constructor_No_Coordinates_Throws_Exception()
         [Test]
         public void Constructor_Null_Coordinates_Throws_Exception()
         {
-            Assert.Throws<ArgumentNullException>(() => new LineString(null));
+            Assert.Throws<ArgumentNullException>(() => new LineString((IEnumerable<IPosition>)null));
         }
 
         private LineString GetLineString(double offset = 0.0)

src/GeoJSON.Net/Converters/CrsConverter.cs

@@ -1,9 +1,6 @@
 // Copyright © Joerg Battermann 2014, Matt Hunt 2017
 
 using System;
-#if (!NET35 || !NET40)
-using System.Reflection;
-#endif
 using GeoJSON.Net.CoordinateReferenceSystem;
 using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
@@ -24,11 +21,7 @@ public class CrsConverter : JsonConverter
         /// </returns>
         public override bool CanConvert(Type objectType)
         {
-#if (NET35 || NET40)
-            return typeof(ICRSObject).IsAssignableFrom(objectType);
-#else
-            return typeof(ICRSObject).GetTypeInfo().IsAssignableFrom(objectType.GetTypeInfo());
-#endif
+            return typeof(ICRSObject).IsAssignableFromType(objectType);
         }
 
         /// <summary>

src/GeoJSON.Net/Converters/GeoJsonConverter.cs

@@ -3,9 +3,6 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
-#if (!NET35 || !NET40)
-using System.Reflection;
-#endif
 using GeoJSON.Net.Feature;
 using GeoJSON.Net.Geometry;
 using Newtonsoft.Json;
@@ -27,11 +24,7 @@ public class GeoJsonConverter : JsonConverter
 		/// </returns>
 		public override bool CanConvert(Type objectType)
 		{
-#if (NET35 || NET40)
-            return typeof(IGeoJSONObject).IsAssignableFrom(objectType);
-#else
-			return typeof(IGeoJSONObject).GetTypeInfo().IsAssignableFrom(objectType.GetTypeInfo());
-#endif
+			return typeof(IGeoJSONObject).IsAssignableFromType(objectType);
 		}
 
 		/// <summary>

src/GeoJSON.Net/Converters/GeometryConverter.cs

@@ -2,10 +2,8 @@
 
 using System;
 using System.Collections.Generic;
+using System.Collections.ObjectModel;
 using System.Linq;
-#if (!NET35 || !NET40)
-using System.Reflection;
-#endif
 using GeoJSON.Net.Geometry;
 using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
@@ -26,11 +24,7 @@ public class GeometryConverter : JsonConverter
         /// </returns>
         public override bool CanConvert(Type objectType)
         {
-#if (NET35 || NET40)
-            return typeof(IGeometryObject).IsAssignableFrom(objectType);
-#else
-            return typeof(IGeometryObject).GetTypeInfo().IsAssignableFrom(objectType.GetTypeInfo());
-#endif
+            return typeof(IGeometryObject).IsAssignableFromType(objectType);
         }
 
         /// <summary>
@@ -54,8 +48,8 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist
                     return ReadGeoJson(value);
                 case JsonToken.StartArray:
                     var values = JArray.Load(reader);
-                    var geometries = new List<IGeometryObject>(values.Count);
-                    geometries.AddRange(values.Cast<JObject>().Select(ReadGeoJson));
+                    var geometries = new ReadOnlyCollection<IGeometryObject>(
+                        values.Cast<JObject>().Select(ReadGeoJson).ToArray());
                     return geometries;
             }
 

…oJSON.Net/Converters/PolygonConverter.cs …verters/LineStringEnumerableConverter.cssrc/GeoJSON.Net/Converters/PolygonConverter.cs renamed to src/GeoJSON.Net/Converters/LineStringEnumerableConverter.cs src/GeoJSON.Net/Converters/PolygonConverter.cs renamed to src/GeoJSON.Net/Converters/LineStringEnumerableConverter.cs

@@ -2,17 +2,19 @@
 
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using GeoJSON.Net.Geometry;
 using Newtonsoft.Json;
+using Newtonsoft.Json.Linq;
 
 namespace GeoJSON.Net.Converters
 {
     /// <summary>
-    /// Converter to read and write the <see cref="Polygon" /> type.
+    /// Converter to read and write the <see cref="IEnumerable{LineString}" /> type.
     /// </summary>
-    public class PolygonConverter : JsonConverter
+    public class LineStringEnumerableConverter : JsonConverter
     {
-        private static readonly LineStringConverter LineStringConverter = new LineStringConverter();
+        private static readonly PositionEnumerableConverter LineStringConverter = new PositionEnumerableConverter();
 
         /// <summary>
         /// Determines whether this instance can convert the specified object type.
@@ -23,7 +25,7 @@ public class PolygonConverter : JsonConverter
         /// </returns>
         public override bool CanConvert(Type objectType)
         {
-            return objectType == typeof(Polygon);
+            return typeof(IEnumerable<LineString>).IsAssignableFromType(objectType);
         }
 
         /// <summary>
@@ -38,16 +40,13 @@ public override bool CanConvert(Type objectType)
         /// </returns>
         public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
         {
-            var rings = serializer.Deserialize<double[][][]>(reader);
-            var lineStrings = new List<LineString>(rings.Length);
-
-            foreach (var ring in rings)
-            {
-                var positions = (IEnumerable<IPosition>)LineStringConverter.ReadJson(reader, typeof(LineString), ring, serializer);
-                lineStrings.Add(new LineString(positions));
-            }
-
-            return lineStrings;
+            var rings = existingValue as JArray ?? serializer.Deserialize<JArray>(reader);
+            return rings.Select(ring => new LineString((IEnumerable<IPosition>) LineStringConverter.ReadJson(
+                    reader,
+                    typeof(IEnumerable<IPosition>),
+                    ring,
+                    serializer)))
+                .ToArray();
         }
 
         /// <summary>
@@ -58,28 +57,18 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist
         /// <param name="serializer">The calling serializer.</param>
         public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
         {
-            var coordinateElements = value as List<LineString>;
-            if (coordinateElements != null && coordinateElements.Count > 0)
+            if (value is IEnumerable<LineString> coordinateElements)
             {
-                if (coordinateElements[0].Coordinates[0] is Position)
-                {
-                    writer.WriteStartArray();
-
-                    foreach (var subPolygon in coordinateElements)
-                    {
-                        LineStringConverter.WriteJson(writer, subPolygon.Coordinates, serializer);
-                    }
-
-                    writer.WriteEndArray();
-                }
-                else
+                writer.WriteStartArray();
+                foreach (var subPolygon in coordinateElements)
                 {
-                    throw new NotImplementedException();
+                    LineStringConverter.WriteJson(writer, subPolygon.Coordinates, serializer);
                 }
+                writer.WriteEndArray();
             }
             else
             {
-                serializer.Serialize(writer, value);
+                throw new ArgumentException($"{nameof(LineStringEnumerableConverter)}: unsupported value type");
             }
         }
     }

src/GeoJSON.Net/Converters/MultiPointConverter.cs

@@ -0,0 +1,49 @@
+// Copyright � Joerg Battermann 2014, Matt Hunt 2017
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using GeoJSON.Net.Geometry;
+using Newtonsoft.Json;
+using Newtonsoft.Json.Linq;
+
+namespace GeoJSON.Net.Converters
+{
+    /// <summary>
+    /// Converter to read and write the <see cref="IEnumerable{Point}" /> type.
+    /// </summary>
+    public class PointEnumerableConverter : JsonConverter
+    {
+        private static readonly PositionConverter PositionConverter = new PositionConverter();
+        /// <inheritdoc />
+        public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
+        {
+            if (value is IEnumerable<Point> points)
+            {
+                writer.WriteStartArray();
+                foreach (var point in points)
+                {
+                    PositionConverter.WriteJson(writer, point.Coordinates, serializer);
+                }
+                writer.WriteEndArray();
+            }
+            else
+            {
+                throw new ArgumentException($"{nameof(PointEnumerableConverter)}: unsupported value {value}");
+            }
+        }
+
+        /// <inheritdoc />
+        public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
+        {
+            var coordinates = existingValue as JArray ?? serializer.Deserialize<JArray>(reader);
+            return coordinates.Select(position => new Point(position.ToObject<IEnumerable<double>>().ToPosition()));
+        }
+
+        /// <inheritdoc />
+        public override bool CanConvert(Type objectType)
+        {
+            return objectType == typeof(IEnumerable<Point>);
+        }
+    }
+}