Merge pull request #7 from tnunnink/ArrayGeneric

Fixing bugs with array type and string type. documentation and testing.

src/Components/Tag.cs

@@ -77,7 +77,7 @@ private Tag(Tag root, LogixMember member, Tag parent) : base(root.Element)
     public override string Name
     {
         get => GetTagName();
-        set => Element.SetValue(value);
+        set => SetValue(value);
     }
 
     /// <summary>

src/LogixExtensions.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Linq.Expressions;
 using System.Net;
 using System.Reflection;
 using System.Text.RegularExpressions;
@@ -637,6 +638,20 @@ public static Dictionary<TagName, List<NeutralText>> ToTagLookup(this IEnumerabl
     /// <remarks>This is to make converting from string to XName concise.</remarks>
     internal static XName XName(this string value) => System.Xml.Linq.XName.Get(value);
 
+    /// <summary>
+    /// A concise method for getting a required attribute value parsed as the specified type from a XElement object.
+    /// </summary>
+    /// <param name="element">The element containing the attribute to retrieve.</param>
+    /// <param name="name">The name of the attribute value to get.</param>
+    /// <typeparam name="T">The type to parse the attribute value as.</typeparam>
+    /// <returns>The value of the element's specified attribute value parsed as the specified generic type parameter.</returns>
+    /// <exception cref="L5XException">No attribute with <c>name</c> exists for the current element.</exception>
+    internal static T Get<T>(this XElement element, XName name)
+    {
+        var value = element.Attribute(name)?.Value;
+        return value is not null ? value.Parse<T>() : throw new L5XException(name, element);
+    }
+
     /// <summary>
     /// Gets the L5X element name for the specified type. 
     /// </summary>
@@ -652,6 +667,42 @@ internal static string L5XType(this Type type)
         return attribute is not null ? attribute.TypeName : type.Name;
     }
 
+    /// <summary>
+    /// Builds a deserialization expression delegate which returns the specified type using the current type information.
+    /// </summary>
+    /// <param name="type">The current type for which to build the expression.</param>
+    /// <typeparam name="TReturn">The return type of the expression delegate.</typeparam>
+    /// <returns>A <see cref="Func{TResult}"/> which accepts a <see cref="XElement"/> and returns the specified
+    /// return type.</returns>
+    /// <remarks>
+    /// This extension is the basis for how we build the deserialization functions using reflection and
+    /// expression trees. Using compiled expression trees is much more efficient that calling the invoke method for a type's
+    /// constructor info obtained via reflection. This method make all the necessary check on the current type, ensuring the
+    /// deserializer delegate will execute without exception.
+    /// </remarks>
+    internal static Func<XElement, TReturn> Deserializer<TReturn>(this Type type)
+    {
+        if (type is null) throw new ArgumentNullException(nameof(type));
+
+        if (type.IsAbstract)
+            throw new ArgumentException($"Can not build deserializer expression for abstract type '{type.Name}'.");
+
+        if (!typeof(TReturn).IsAssignableFrom(type))
+            throw new ArgumentException(
+                $"The type {type.Name} is not assignable (inherited) from '{typeof(TReturn).Name}'.");
+
+        var constructor = type.GetConstructor(new[] { typeof(XElement) });
+
+        if (constructor is null || !constructor.IsPublic)
+            throw new ArgumentException(
+                $"Can not build expression for type '{type.Name}' without public constructor accepting a XElement parameter.");
+
+        var parameter = Expression.Parameter(typeof(XElement), "element");
+        var factory = Expression.New(constructor, parameter);
+        var lambda = Expression.Lambda(factory, parameter);
+        return (Func<XElement, TReturn>)lambda.Compile();
+    }
+
     /// <summary>
     /// Determines if a type is derived from the base type, even if the base type is a generic type.
     /// </summary>

src/LogixMember.cs

@@ -282,9 +282,9 @@ private static XElement SerializeStringMember(string name, StringType type)
     private void OnDataTypeChanged(object sender, EventArgs e)
     {
         //If the sender is an atomic type (which is intercepted by atomic types)
-        //  then we realize this is a value change and need to replace the member type with the new value.
+        //then we realize this is a value change and need to replace the member type with the new value.
         //This is the only way we can update the atomic value on the L5X for a bit member change, since
-        //  atomic bit members don't exist in the L5X data structure.
+        //atomic bit members don't exist in the L5X data structure.
         //Note that setting DataType this way will in turn trigger the member's data changed event. 
         if (sender is AtomicType atomicType)
         {

src/LogixSerializer.cs

@@ -11,23 +11,11 @@ namespace L5Sharp;
 /// </summary>
 public static class LogixSerializer
 {
-    private static readonly Lazy<Dictionary<Type, ConstructorInfo>> Constructors = new(() =>
-    {
-        var dictionary = new Dictionary<Type, ConstructorInfo>();
-
-        var types = typeof(LogixSerializer).Assembly.GetTypes().Where(t =>
-            t.IsDerivativeOf(typeof(LogixElement))
-            && t is { IsAbstract: false, IsPublic: true }
-            && t.GetConstructor(new[] { typeof(XElement) }) is not null);
-
-        foreach (var type in types)
-        {
-            var constructor = type.GetConstructor(new[] { typeof(XElement) });
-            dictionary.TryAdd(type, constructor);
-        }
-
-        return dictionary;
-    });
+    /// <summary>
+    /// The global cache for all <see cref="LogixElement"/> object deserializer delegate functions.
+    /// </summary>
+    private static readonly Lazy<Dictionary<Type, Func<XElement, LogixElement>>> Deserializers = new(() =>
+        Introspect(typeof(LogixSerializer).Assembly).ToDictionary(k => k.Key, v => v.Value));
 
     /// <summary>
     /// Deserializes a <see cref="XElement"/> into the specified object type.
@@ -36,10 +24,11 @@ public static class LogixSerializer
     /// <typeparam name="TElement">The return type of the deserialized element.</typeparam>
     /// <returns>A new object of the specified type representing the deserialized element.</returns>
     /// <remarks>
-    /// The return object must specify a constructor accepting a single <see cref="XElement"/> for deserialization to work.
+    /// The return object must specify a public constructor accepting a <see cref="XElement"/> parameter for
+    /// deserialization to work.
     /// </remarks>
     public static TElement Deserialize<TElement>(XElement element) where TElement : LogixElement =>
-        (TElement)Constructor(typeof(TElement)).Invoke(new object[] { element });
+        (TElement)Deserializer(typeof(TElement)).Invoke(element);
 
     /// <summary>
     /// Deserializes a <see cref="XElement"/> into the specified object type.
@@ -50,30 +39,42 @@ public static TElement Deserialize<TElement>(XElement element) where TElement :
     /// <remarks>
     /// The return object must specify a constructor accepting a single <see cref="XElement"/> for deserialization to work.
     /// </remarks>
-    public static object Deserialize(Type type, XElement element) => Constructor(type).Invoke(new object[] { element });
+    public static object Deserialize(Type type, XElement element) => Deserializer(type).Invoke(element);
 
     /// <summary>
-    /// Handles getting the constructor for the specified type. If the type is not cached, this method will check
+    /// Handles getting the deserializer delegate for the specified type. If the type is not cached, this method will check
     /// if the type inherits <see cref="LogixElement"/> and has a valid constructor. If so, it will add to the
-    /// global constructor cache and return the <see cref="ConstructorInfo"/> object.
+    /// global deserializer cache and return the deserializer delegate function.
     /// </summary>
-    private static ConstructorInfo Constructor(Type type)
+    private static Func<XElement, LogixElement> Deserializer(Type type)
     {
-        if (Constructors.Value.TryGetValue(type, out var constructor))
-            return constructor;
-
-        if (!type.IsDerivativeOf(typeof(LogixElement)))
-            throw new ArgumentException(
-                $"LogixSerializer is only compatible for types inheriting from {typeof(LogixElement)}");
+        if (Deserializers.Value.TryGetValue(type, out var cached))
+            return cached;
 
-        var info = type.GetConstructor(new[] { typeof(XElement) });
+        var deserializer = type.Deserializer<LogixElement>();
+        if (!Deserializers.Value.TryAdd(type, deserializer))
+            throw new InvalidOperationException($"The type {type.Name} is already registered.");    
+        return deserializer;
+    }
 
-        if (info is null)
-            throw new InvalidOperationException(
-                @$"No element constructor defined for type {type}.
-                     Class must specify constructor accepting a single {typeof(XElement)} to be deserialized.");
+    /// <summary>
+    /// Performs reflection scanning of provided <see cref="Assembly"/> to get all public non abstract types
+    /// inheriting from <see cref="LogixElement"/> that have the supported deserialization constructor,
+    /// and returns the <c>L5XType</c> and compiled deserialization delegate pair. This is used to initialize the
+    /// set of concrete deserializer functions for all known logix element objects.
+    /// </summary>
+    private static IEnumerable<KeyValuePair<Type, Func<XElement, LogixElement>>> Introspect(Assembly assembly)
+    {
+        var types = assembly.GetTypes().Where(t =>
+            typeof(LogixElement).IsDerivativeOf(t)
+            && t is { IsAbstract: false, IsPublic: true }
+            && t.GetConstructor(BindingFlags.Public | BindingFlags.Instance, null, new[] { typeof(XElement) },
+                null) is not null);
 
-        Constructors.Value.TryAdd(type, info);
-        return info;
+        foreach (var type in types)
+        {
+            var deserializer = type.Deserializer<LogixElement>();
+            yield return new KeyValuePair<Type, Func<XElement, LogixElement>>(type, deserializer);
+        }
     }
 }

src/LogixType.cs

@@ -102,8 +102,10 @@ public override bool Equals(object obj)
         Members.SingleOrDefault(m => string.Equals(m.Name, name, StringComparison.OrdinalIgnoreCase));
 
     /// <summary>
-    /// Handles raising the <see cref="DataChanged"/> event for the type with the provided object sender.
+    /// Raising the <see cref="DataChanged"/> event for the type with the provided object sender.
     /// </summary>
+    /// <param name="sender">The objet initiating the data changed event. This could be this object, or a descendent
+    /// member or type in the data hierarchy.</param>
     protected void RaiseDataChanged(object sender) => DataChanged?.Invoke(sender, EventArgs.Empty);
 
     /// <inheritdoc />

src/Types/ArrayType.cs

@@ -45,6 +45,7 @@ public ArrayType(Array array)
             throw new ArgumentException("Array can not be initialized with different logix type items.", nameof(array));
 
         Dimensions = Dimensions.FromArray(array);
+
         _members = Dimensions.Indices().Zip(collection, (i, t) =>
         {
             var member = new LogixMember(i, t);
@@ -63,8 +64,8 @@ public ArrayType(XElement element)
     {
         if (element is null) throw new ArgumentNullException(nameof(element));
 
-        Dimensions = element.Attribute(L5XName.Dimensions)?.Value.Parse<Dimensions>() ??
-                     throw new L5XException(L5XName.Dimensions, element);
+        Dimensions = element.Get<Dimensions>(L5XName.Dimensions);
+
         _members = element.Elements().Select(e =>
         {
             var member = new LogixMember(e);
@@ -73,7 +74,17 @@ public ArrayType(XElement element)
         }).ToList();
     }
 
+    /// <summary>
+    /// The name of the logix type the array contains.
+    /// </summary>
+    /// <value>A <see cref="string"/> containing the text name of the logix type for which the array contains.</value>
+    /// <remarks>This is used to delineate from the <see cref="Name"/> property of the array type which is the type name and
+    /// the dimensions index concatenated. The type name is used for serialization.</remarks>
+    public string TypeName => this.First().Name;
+
     /// <inheritdoc />
+    /// <remarks>The name of an array type will be the name of it's contained element types with the dimensions index
+    /// text appended. This helps differentiate types when querying so we don't return arrays and type</remarks>
     public override string Name => $"{this.First().Name}{Dimensions.ToIndex()}";
 
     /// <inheritdoc />
@@ -83,7 +94,7 @@ public ArrayType(XElement element)
     public override DataTypeClass Class => this.First().Class;
 
     /// <summary>
-    /// Gets the dimensions of the the array.
+    /// The dimensions of the the array, which defined the length and rank of the array's elements.
     /// </summary>
     /// <value>A <see cref="Core.Dimensions"/> value representing the array dimensions.</value>
     public Dimensions Dimensions { get; }
@@ -97,17 +108,12 @@ public ArrayType(XElement element)
 
     /// <inheritdoc />
     public override IEnumerable<LogixMember> Members => _members.AsEnumerable();
-
-    /// <summary>
-    /// 
-    /// </summary>
-    public string TypeName => this.First().Name;
 
     /// <inheritdoc />
     public override XElement Serialize()
     {
         var element = new XElement(L5XName.Array);
-        element.Add(new XAttribute(L5XName.DataType, this.First().Name));
+        element.Add(new XAttribute(L5XName.DataType, TypeName));
         element.Add(new XAttribute(L5XName.Dimensions, Dimensions));
         if (Radix != Radix.Null) element.Add(new XAttribute(L5XName.Radix, Radix));
         element.Add(_members.Select(m =>

src/Types/Predefined/ALARM.cs

@@ -1,4 +1,5 @@
-using L5Sharp.Enums;
+using System.Xml.Linq;
+using L5Sharp.Enums;
 using L5Sharp.Types.Atomics;
 
 // ReSharper disable InconsistentNaming I want to keep the naming consistent with Logix (for now).
@@ -40,6 +41,11 @@ public ALARM() : base(nameof(ALARM))
         ROCNegLimitInv = new BOOL();
         ROCPeriodInv = new BOOL();
     }
+
+    /// <inheritdoc />
+    public ALARM(XElement element) : base(element)
+    {
+    }
 
     /// <inheritdoc />
     public override DataTypeClass Class => DataTypeClass.Predefined;

src/Types/Predefined/STRING.cs

@@ -20,7 +20,7 @@ public STRING() : base(nameof(STRING), string.Empty)
     }
 
     /// <inheritdoc />
-    public STRING(XElement element) : base(element)
+    public STRING(XElement element) : base(element, PredefinedLength)
     {
     }