Update Feature constructor definition to use interface instead of concrete class

Feature defines a constructor with IGeometryObject, Dictionary<string, object>, string types. If an IDictionary<string, object> is provided it will use another Feature constructor defined with IGeometryObject, object, string and will use reflection to create a Dictionary implementation.

When this constructor is used with an IDictionary<string, object> a Runtime Exception is thrown with the message: Parameter count mismatch.

The first constructor should be updated to follow the Liskov Substitution Principle (https://en.wikipedia.org/wiki/Liskov_substitution_principle) to prevent this undesired behavior:

> Substitutability is a principle in object-oriented programming stating that, in a computer program, if S is a subtype of T, then objects of type T may be replaced with objects of type S (i.e. an object of type T may be substituted with any object of a subtype S) without altering any of the desirable properties of the program (correctness, task performed, etc.).

Unit tests were added to verify that the behavior does not change when the interface is used instead of the concrete implementation.

Resolves: #116

Author: justinhachemeister

src/GeoJSON.Net.Tests/Feature/FeatureTests.cs

@@ -167,6 +167,28 @@ public void Can_Serialize_MultiPolygon_Feature()
             JsonAssert.AreEqual(expectedJson, actualJson);
         }
 
+        [Test]
+        public void Can_Serialize_Dictionary_Subclass()
+        {
+            var properties =
+                new TestFeaturePropertyDictionary()
+                {
+                     BooleanProperty = true,
+                     DoubleProperty = 1.2345d,
+                     EnumProperty = TestFeatureEnum.Value1,
+                     IntProperty = -1,
+                     StringProperty = "Hello, GeoJSON !"
+                };
+
+            Net.Feature.Feature feature = new Net.Feature.Feature(new Point(new Position(10, 10)), properties);
+
+            var expectedJson = this.GetExpectedJson();
+            var actualJson = JsonConvert.SerializeObject(feature);
+
+            Assert.False(string.IsNullOrEmpty(expectedJson));
+            JsonAssert.AreEqual(expectedJson, actualJson);
+        }
+
         [Test]
         public void Ctor_Can_Add_Properties_Using_Object()
         {
@@ -187,6 +209,31 @@ public void Ctor_Can_Add_Properties_Using_Object()
             Assert.AreEqual(feature.Properties.Count, 6);
         }
 
+        [Test]
+        public void Ctor_Can_Add_Properties_Using_Object_Inheriting_Dictionary()
+        {
+            int expectedProperties = 6;
+
+            var properties = new TestFeaturePropertyDictionary()
+            {
+                BooleanProperty = true,
+                DateTimeProperty = DateTime.Now,
+                DoubleProperty = 1.2345d,
+                EnumProperty = TestFeatureEnum.Value1,
+                IntProperty = -1,
+                StringProperty = "Hello, GeoJSON !"
+            };
+
+            Net.Feature.Feature feature = new Net.Feature.Feature(new Point(new Position(10, 10)), properties);
+
+            Assert.IsNotNull(feature.Properties);
+            Assert.IsTrue(feature.Properties.Count > 1);
+            Assert.AreEqual(
+                feature.Properties.Count,
+                expectedProperties,
+                $"Expected: {expectedProperties} Actual: {feature.Properties.Count}");
+        }
+
         [Test]
         public void Ctor_Creates_Properties_Collection_When_Passed_Null_Proper_Object()
         {
@@ -437,7 +484,7 @@ private IGeometryObject GetGeometry()
             return multiLine;
         }
 
-        public static Dictionary<string, object> GetPropertiesInRandomOrder()
+        public static IDictionary<string, object> GetPropertiesInRandomOrder()
         {
             var properties = new Dictionary<string, object>()
             {

src/GeoJSON.Net.Tests/Feature/FeatureTests_Can_Serialize_Dictionary_Subclass.json

@@ -0,0 +1,17 @@
+{
+  "type":"Feature",
+  "geometry":{
+    "type":"Point",
+    "coordinates":[
+      10.0,
+      10.0
+    ]
+  },
+  "properties":{
+    "BooleanProperty":true,
+    "DoubleProperty":1.2345,
+    "EnumProperty":1,
+    "IntProperty":-1,
+    "StringProperty":"Hello, GeoJSON !"
+  }
+}

src/GeoJSON.Net.Tests/Feature/TestFeaturePropertyDictionary.cs

@@ -0,0 +1,230 @@
+namespace GeoJSON.Net.Tests.Feature
+{
+    using System;
+    using System.Collections;
+    using System.Collections.Generic;
+
+    /// <summary>
+    /// The Test Property Dictionary object.
+    /// </summary>
+    internal class TestFeaturePropertyDictionary : IDictionary<string, object>
+    {
+        /// <summary>
+        /// The internal dictionary this implementation is wrapping for testing purposes.
+        /// </summary>
+        private readonly IDictionary<string, object> internalDictionary;
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="TestFeaturePropertyDictionary"/> class.
+        /// </summary>
+        public TestFeaturePropertyDictionary()
+        {
+            this.internalDictionary = new Dictionary<string, object>();
+        }
+
+        public bool BooleanProperty
+        {
+            get
+            {
+                return this.GetKeyOrDefault<bool>(nameof(this.BooleanProperty));
+            }
+
+            set
+            {
+                this.internalDictionary[nameof(this.BooleanProperty)] = value;
+            }
+        }
+
+        public DateTime DateTimeProperty
+        {
+            get
+            {
+                return this.GetKeyOrDefault<DateTime>(nameof(this.DateTimeProperty));
+            }
+
+            set
+            {
+                this.internalDictionary[nameof(this.DateTimeProperty)] = value;
+            }
+        }
+
+        public double DoubleProperty
+        {
+            get
+            {
+                return this.GetKeyOrDefault<double>(nameof(this.DoubleProperty));
+            }
+
+            set
+            {
+                this.internalDictionary[nameof(this.DoubleProperty)] = value;
+            }
+        }
+
+        public TestFeatureEnum EnumProperty
+        {
+            get
+            {
+                return this.GetKeyOrDefault<TestFeatureEnum>(nameof(this.EnumProperty));
+            }
+
+            set
+            {
+                this.internalDictionary[nameof(this.EnumProperty)] = value;
+            }
+        }
+
+        public int IntProperty
+        {
+            get
+            {
+                return this.GetKeyOrDefault<int>(nameof(this.IntProperty));
+            }
+
+            set
+            {
+                this.internalDictionary[nameof(this.IntProperty)] = value;
+            }
+        }
+
+        public string StringProperty
+        {
+            get
+            {
+                return this.GetKeyOrDefault<string>(nameof(this.StringProperty));
+            }
+
+            set
+            {
+                this.internalDictionary[nameof(this.StringProperty)] = value;
+            }
+        }
+
+        /// <inheritdoc/>
+        public int Count
+        {
+            get
+            {
+                return this.internalDictionary.Count;
+            }
+        }
+
+        /// <inheritdoc/>
+        public bool IsReadOnly
+        {
+            get
+            {
+                return this.internalDictionary.IsReadOnly;
+            }
+        }
+
+        /// <inheritdoc/>
+        public ICollection<string> Keys
+        {
+            get
+            {
+                return this.internalDictionary.Keys;
+            }
+        }
+
+        /// <inheritdoc/>
+        public ICollection<object> Values
+        {
+            get
+            {
+                return this.internalDictionary.Values;
+            }
+        }
+
+        /// <inheritdoc/>
+        public object this[string key]
+        {
+            get
+            {
+                return this.internalDictionary[key];
+            }
+
+            set
+            {
+                this.internalDictionary[key] = value;
+            }
+        }
+
+        /// <inheritdoc/>
+        public IEnumerator<KeyValuePair<string, object>> GetEnumerator()
+        {
+            return this.internalDictionary.GetEnumerator();
+        }
+
+        /// <inheritdoc/>
+        IEnumerator IEnumerable.GetEnumerator()
+        {
+            return this.GetEnumerator();
+        }
+
+        /// <inheritdoc/>
+        public void Add(KeyValuePair<string, object> item)
+        {
+            this.internalDictionary.Add(item);
+        }
+
+        /// <inheritdoc/>
+        public void Clear()
+        {
+            this.internalDictionary.Clear();
+        }
+
+        /// <inheritdoc/>
+        public bool Contains(KeyValuePair<string, object> item)
+        {
+            return this.internalDictionary.Contains(item);
+        }
+
+        /// <inheritdoc/>
+        public void CopyTo(KeyValuePair<string, object>[] array, int arrayIndex)
+        {
+            this.internalDictionary.CopyTo(array, arrayIndex);
+        }
+
+        /// <inheritdoc/>
+        public bool Remove(KeyValuePair<string, object> item)
+        {
+            return this.internalDictionary.Remove(item);
+        }
+
+        /// <inheritdoc/>
+        public bool ContainsKey(string key)
+        {
+            return this.internalDictionary.ContainsKey(key);
+        }
+
+        /// <inheritdoc/>
+        public void Add(string key, object value)
+        {
+            this.internalDictionary.Add(key, value);
+        }
+
+        /// <inheritdoc/>
+        public bool Remove(string key)
+        {
+            return this.internalDictionary.Remove(key);
+        }
+
+        /// <inheritdoc/>
+        public bool TryGetValue(string key, out object value)
+        {
+            return this.internalDictionary.TryGetValue(key, out value);
+        }
+
+        private T GetKeyOrDefault<T>(string keyName)
+        {
+            object value;
+            if (this.TryGetValue(keyName, out value))
+            {
+                return (T)value;
+            }
+
+            return default(T);
+        }
+    }
+}

src/GeoJSON.Net/Feature/Feature.cs

@@ -105,7 +105,7 @@ public override int GetHashCode()
     public class Feature : Feature<IGeometryObject>
     {
         [JsonConstructor]
-        public Feature(IGeometryObject geometry, Dictionary<string, object> properties = null, string id = null) 
+        public Feature(IGeometryObject geometry, IDictionary<string, object> properties = null, string id = null) 
             : base(geometry, properties, id)
         {
         }
@@ -122,7 +122,7 @@ public Feature(IGeometryObject geometry, object properties, string id = null)
     /// </summary>
     /// <remarks>Returns correctly typed Geometry property</remarks>
     /// <typeparam name="TGeometry"></typeparam>
-    public class Feature<TGeometry> : Feature<TGeometry, Dictionary<string, object>>, IEquatable<Feature<TGeometry>> where TGeometry : IGeometryObject
+    public class Feature<TGeometry> : Feature<TGeometry, IDictionary<string, object>>, IEquatable<Feature<TGeometry>> where TGeometry : IGeometryObject
     {
 
         /// <summary>
@@ -132,7 +132,7 @@ public class Feature<TGeometry> : Feature<TGeometry, Dictionary<string, object>>
         /// <param name="properties">The properties.</param>
         /// <param name="id">The (optional) identifier.</param>
         [JsonConstructor]
-        public Feature(TGeometry geometry, Dictionary<string, object> properties = null, string id = null)
+        public Feature(TGeometry geometry, IDictionary<string, object> properties = null, string id = null)
         : base(geometry, properties ?? new Dictionary<string, object>(), id)
         {
         }