Merge pull request #2973 from FirelyTeam/6.0/remove-idictionary

Remove Children, NamedChildren and IDictionary

src/Hl7.Fhir.Base/ElementModel/NewPocoBuilder.cs

@@ -40,7 +40,7 @@ public Base BuildFrom(ITypedElement source)
 
     private Base readFromElement(ITypedElement node, ClassMapping classMapping)
     {
-        IDictionary<string, object> newInstance = buildNewInstance(classMapping);
+        var newInstance = buildNewInstance(classMapping);
 
         // Capture the instance type if this is a dynamic type.
         if(newInstance is IDynamicType dt)
@@ -79,30 +79,28 @@ private Base readFromElement(ITypedElement node, ClassMapping classMapping)
             var childClassMapping = classMappingForElement(child, propertyMapping);
             var convertedValue = readFromElement(child, childClassMapping);
 
-
             // In case the convertedValue does not agree with the actual POCO type of the property, this
             // method will throw an InvalidCastException. Later, we could salvage
             // the data we have so far, and put it in an annotation.
             // This will be fixed in https://github.com/FirelyTeam/firely-net-sdk/issues/2908.
             setOrAddProperty(child, newInstance, convertedValue, propertyMapping);
         }
 
-        return (Base)newInstance;
+        return newInstance;
     }
 
     private static void raiseFormatError(string message, string location)
     {
         throw Error.Format("While building a POCO: " + message, location);
     }
 
-    private static IDictionary<string, object> buildNewInstance(ClassMapping mapping)
+    private static Base buildNewInstance(ClassMapping mapping)
     {
-        return mapping.Factory() switch
-        {
-            IDictionary<string,object> b => b,
-            _ => throw Error.InvalidOperation($"Class Factory for '{mapping.Name}' did not return a dictionary, which is required for " +
-                        $"building up POCO's dynamically.")
-        };
+        if (mapping.Factory() is Base b) return b;
+
+        throw Error.InvalidOperation($"Class Factory for '{mapping.Name}' did not return a " +
+                       $"Base, which is required for " +
+                        $"building up POCO's dynamically.");
     }
 
     private IList buildNewList(PropertyMapping? propertyMapping, Type elementType)
@@ -118,7 +116,7 @@ private IList buildNewList(PropertyMapping? propertyMapping, Type elementType)
         }
 
         var propertyClassMapping = getClassMapping(propertyMapping.ImplementingType);
-        return propertyClassMapping?.ListFactory() ?? new List<Base>();
+        return propertyClassMapping.ListFactory() ?? new List<Base>();
     }
 
     private ClassMapping classMappingForElement(ITypedElement node, PropertyMapping? propertyMapping)
@@ -212,7 +210,7 @@ private ClassMapping getClassMapping(Type t) =>
 
     private ClassMapping getClassMapping<T>() => getClassMapping(typeof(T));
 
-    private void setOrAddProperty(ITypedElement node, IDictionary<string, object> target,
+    private void setOrAddProperty(ITypedElement node, Base target,
         Base convertedValue, PropertyMapping? propertyMapping)
     {
         // If this element *could* be repeating (either we don't know the definition, or it really is defined
@@ -291,7 +289,7 @@ private void setOrAddProperty(ITypedElement node, IDictionary<string, object> ta
     /// <summary>
     /// Convert the value of a typed element to a value that can be set on a POCO property.
     /// </summary>
-    private object convertTypedElementValue(object value, string? instanceType)
+    private static object convertTypedElementValue(object value, string? instanceType)
     {
         return value switch
         {

src/Hl7.Fhir.Base/ElementModel/PocoElementNode.cs

@@ -75,9 +75,9 @@ private Type determineInstanceType(PropertyMapping definition)
             };
         }
 
-        public IElementDefinitionSummary Definition { get; private set; }
+        public IElementDefinitionSummary Definition { get; }
 
-        public string ShortPath { get; private set; }
+        public string ShortPath { get; }
 
         /// <summary>
         /// Elements from the IReadOnlyDictionary can be of type <see cref="Base"/>, IEnumerable&lt;Base&gt; or string.
@@ -107,15 +107,13 @@ public IEnumerable<ITypedElement> Children(string name)
         {
             if (Current is null) return Enumerable.Empty<PocoElementNode>();
 
-            var rod = Current.AsReadOnlyDictionary();
-
             if (name is null)
             {
-                return rod.SelectMany(kvp
+                return Current.GetElementPairs().SelectMany(kvp
                     => createChildNodes(kvp.Key, kvp.Value));
             }
 
-            rod.TryGetValue(name, out var dictValue);
+            Current.TryGetValue(name, out var dictValue);
             return createChildNodes(name, dictValue);
 
             IEnumerable<PocoElementNode> createChildNodes(string childName, object value)
@@ -185,18 +183,13 @@ public object Value
         {
             get
             {
-                if (Current is PrimitiveType p && p.ObjectValue != null)
-                {
-                    if (p.ObjectValue != _lastCachedValue)
-                    {
-                        _value = ToITypedElementValue();
-                        _lastCachedValue = p.ObjectValue;
-                    }
-
-                    return _value;
-                }
-                else
-                    return null;
+                if (Current is not PrimitiveType { ObjectValue: not null } p) return null;
+
+                if (p.ObjectValue == _lastCachedValue) return _value;
+
+                _value = ToITypedElementValue();
+                _lastCachedValue = p.ObjectValue;
+                return _value;
             }
         }
 

src/Hl7.Fhir.Base/Model/Base.Dictionary.cs

@@ -7,87 +7,43 @@
 
 namespace Hl7.Fhir.Model;
 
-public abstract partial class Base: IReadOnlyDictionary<string,object>, IDictionary<string, object>
+public class BaseDictionary(Base wrapped) : IReadOnlyDictionary<string,object>
 {
-    private object get(string key) =>
-        this.TryGetValue(key, out var value)
-            ? value
-            : throw new KeyNotFoundException($"Element '{key}' is not a known FHIR element or has no value.");
-
-    public IReadOnlyDictionary<string, object> AsReadOnlyDictionary() => this;
-    public IDictionary<string, object> AsDictionary() => this;
+    private static object wrap(object value) =>
+        value switch
+        {
+            Base b => new BaseDictionary(b),
+            _ => value
+        };
 
     #region IReadOnlyDictionary
 
-    IEnumerable<string> IReadOnlyDictionary<string, object>.Keys => GetElementPairs().Select(kvp => kvp.Key);
-    IEnumerable<object> IReadOnlyDictionary<string, object>.Values => GetElementPairs().Select(kvp => kvp.Value);
-    int IReadOnlyCollection<KeyValuePair<string, object>>.Count => GetElementPairs().Count();
-    object IReadOnlyDictionary<string, object>.this[string key] => get(key);
-
-    bool IReadOnlyDictionary<string, object>.TryGetValue(string key, out object value) =>
-        TryGetValue(key, out value!);
-
-    bool IReadOnlyDictionary<string, object>.ContainsKey(string key) => TryGetValue(key, out _);
-
-    IEnumerator<KeyValuePair<string, object>> IEnumerable<KeyValuePair<string, object>>.GetEnumerator() =>
-        GetElementPairs().GetEnumerator();
-
-    IEnumerator IEnumerable.GetEnumerator() => GetElementPairs().GetEnumerator();
-
-    #endregion
-
-    #region IDictionary
-
-    void ICollection<KeyValuePair<string, object>>.Add(KeyValuePair<string, object> item) =>
-        AsDictionary().Add(item.Key, item.Value);
-
-    void ICollection<KeyValuePair<string, object>>.Clear()
-    {
-        // Slow....
-        foreach (var kvp in this)
-            SetValue(kvp.Key, null);
-    }
+    IEnumerable<string> IReadOnlyDictionary<string, object>.Keys => wrapped.GetElementPairs().Select(kvp => kvp.Key);
+    IEnumerable<object> IReadOnlyDictionary<string, object>.Values => wrapped.GetElementPairs().Select(kvp => wrap(kvp.Value));
+    int IReadOnlyCollection<KeyValuePair<string, object>>.Count => wrapped.GetElementPairs().Count();
 
-    bool ICollection<KeyValuePair<string, object>>.Contains(KeyValuePair<string, object> item) =>
-        TryGetValue(item.Key, out _); // we don't care about the item, we cannot have the same key twice.
+    object IReadOnlyDictionary<string, object>.this[string key] => wrap(wrapped[key]);
 
-    void ICollection<KeyValuePair<string, object>>.CopyTo(KeyValuePair<string, object>[] array, int arrayIndex)
+    bool IReadOnlyDictionary<string, object>.TryGetValue(string key, out object value)
     {
-        foreach (var kvp in this)
-            array[arrayIndex++] = kvp;
+        if (wrapped.TryGetValue(key, out var unwrapped))
+        {
+            value = wrap(unwrapped);
+            return true;
+        }
+
+        value = null!;
+        return false;
     }
 
-    bool ICollection<KeyValuePair<string, object>>.Remove(KeyValuePair<string, object> item) =>
-        AsDictionary().Remove(item.Key);
+    bool IReadOnlyDictionary<string, object>.ContainsKey(string key) => wrapped.TryGetValue(key, out _);
 
-    int ICollection<KeyValuePair<string, object>>.Count => AsReadOnlyDictionary().Count;
-    bool ICollection<KeyValuePair<string, object>>.IsReadOnly => false;
-    ICollection<object> IDictionary<string, object>.Values => AsReadOnlyDictionary().Values.ToList();
-    ICollection<string> IDictionary<string, object>.Keys => AsReadOnlyDictionary().Keys.ToList();
-    bool IDictionary<string, object>.TryGetValue(string key, out object value) => TryGetValue(key, out value!);
-
-    object IDictionary<string, object>.this[string key]
-    {
-        get => this.AsReadOnlyDictionary()[key];
-        set => SetValue(key, value);
-    }
-
-    void IDictionary<string, object>.Add(string key, object value)
-    {
-        if (TryGetValue(key, out _))
-            throw new ArgumentException($"An element with the key '{key}' already exists in the dictionary.");
-
-        SetValue(key, value);
-    }
-
-    bool IDictionary<string, object>.ContainsKey(string key) => TryGetValue(key, out _);
+    IEnumerator<KeyValuePair<string, object>> IEnumerable<KeyValuePair<string, object>>.GetEnumerator() =>
+        wrapped.GetElementPairs().Select(kvp => KeyValuePair.Create(kvp.Key, wrap(kvp.Value))).GetEnumerator();
 
-    bool IDictionary<string, object>.Remove(string key)
-    {
-        var existed = TryGetValue(key, out _);
-        SetValue(key, null);
-        return existed;
-    }
+    IEnumerator IEnumerable.GetEnumerator() =>
+        wrapped.GetElementPairs().Select(kvp => KeyValuePair.Create(kvp.Key, wrap(kvp.Value))).GetEnumerator();
 
     #endregion
+
 }

src/Hl7.Fhir.Base/Model/Base.Extensions.cs

@@ -0,0 +1,39 @@
+#nullable enable
+using System;
+using System.Collections;
+using System.Collections.Generic;
+
+namespace Hl7.Fhir.Model;
+
+public static class BaseExtensions
+{
+    [Obsolete("Use GetElementPairs() instead. Note that with GetElementPairs(), the elements are not guaranteed to " +
+              "be the same type, as they reflect the type in the actual POCO definition.")]
+    public static IEnumerable<Base> Children(this Base instance)
+    {
+        foreach (var element in instance.GetElementPairs())
+        {
+            switch (element.Key, element.Value)
+            {
+                case ("div", XHtml xhtml):
+                    yield return new FhirString(xhtml.Value);
+                    break;
+                case ("id", string id):
+                    yield return new FhirString(id);
+                    break;
+                case ("url", string url):
+                    yield return new FhirUri(url);
+                    break;
+                case (_, IEnumerable<Base> list):
+                    foreach (var item in list)
+                        yield return item;
+                    break;
+                case("value", _) when instance is PrimitiveType:
+                    yield break;
+                default:
+                    yield return (Base)element.Value;
+                    break;
+            }
+        }
+    }
+}

src/Hl7.Fhir.Base/Model/Base.cs

@@ -33,11 +33,11 @@ POSSIBILITY OF SUCH DAMAGE.
 using Hl7.Fhir.Introspection;
 using Hl7.Fhir.Utility;
 using System;
-using System.Collections;
 using System.Collections.Generic;
 using System.ComponentModel;
 using System.ComponentModel.DataAnnotations;
 using System.Diagnostics.CodeAnalysis;
+using System.Linq;
 using System.Runtime.Serialization;
 using System.Threading;
 
@@ -95,7 +95,7 @@ protected virtual void OnPropertyChanged(string property) =>
 
     #endregion
 
-    protected virtual Base SetValue(string key, object? value)
+    internal protected virtual Base SetValue(string key, object? value)
     {
         if (value is null)
             Overflow.Remove(key);
@@ -105,10 +105,18 @@ protected virtual Base SetValue(string key, object? value)
         return this;
     }
 
-    protected virtual bool TryGetValue(string key, [NotNullWhen(true)] out object? value) =>
+    internal object this[string key]
+    {
+        get => this.TryGetValue(key, out var value)
+            ? value
+            : throw new KeyNotFoundException($"Element '{key}' is not a known FHIR element or has no value.");
+        set => SetValue(key, value);
+    }
+
+    internal protected virtual bool TryGetValue(string key, [NotNullWhen(true)] out object? value) =>
         Overflow.TryGetValue(key, out value);
 
-    protected virtual IEnumerable<KeyValuePair<string, object>> GetElementPairs() => Overflow;
+    internal protected virtual IEnumerable<KeyValuePair<string, object>> GetElementPairs() => Overflow;
 
     // TODO bring Children + NamedChildren over as well.
 }

src/Hl7.Fhir.Base/Model/DynamicDataType.cs

@@ -11,8 +11,6 @@ namespace Hl7.Fhir.Model;
 public interface IDynamicType
 {
     public string? DynamicTypeName { get; set; }
-
-    public object this[string key] { get; set; }
 }
 
 /// <summary>
@@ -26,15 +24,6 @@ public class DynamicDataType : DataType, IDynamicType
     public string? DynamicTypeName { get; set; }
 
     public override string TypeName => DynamicTypeName ?? base.TypeName;
-
-    public void Add(string arg1, object arg2) => this.SetValue(arg1, arg2);
-
-    // TODO: One may wonder whether normal resources should have this as well.
-    public object this[string key]
-    {
-        get => this.AsReadOnlyDictionary()[key];
-        set => SetValue(key, value);
-    }
 }
 
 
@@ -50,15 +39,6 @@ public class DynamicResource : Resource, IDynamicType
     public string? DynamicTypeName { get; set; }
 
     public override string TypeName => DynamicTypeName ?? base.TypeName;
-
-    public void Add(string arg1, object arg2) => this.SetValue(arg1, arg2);
-
-    // TODO: One may wonder whether normal resources should have this as well.
-    public object this[string key]
-    {
-        get => this.AsReadOnlyDictionary()[key];
-        set => SetValue(key, value);
-    }
 }
 
 
@@ -73,13 +53,4 @@ public class DynamicPrimitive : PrimitiveType, IDynamicType
     public string? DynamicTypeName { get; set; }
 
     public override string TypeName => DynamicTypeName ?? base.TypeName;
-
-    public void Add(string arg1, object arg2) => this.SetValue(arg1, arg2);
-
-    // TODO: One may wonder whether normal resources should have this as well.
-    public object this[string key]
-    {
-        get => this.AsReadOnlyDictionary()[key];
-        set => SetValue(key, value);
-    }
 }