Skip to content

Commit d0fb8a4

Browse files
committed
Moved IsList and IsConvertible to ItemMetadata properties
With `ItemMetadata` incorporated into `ItemConfiguration`, any method which relies on `ItemConfiguration` or `PropertyConfiguration` now has access to metadata that's shared between parameters (via `ParameterMetadata`) or methods or properties (via `MemberAccessor`). Given that, we can move `IsList()` to `ItemMetadata`, and cache the values of both `IsList` and `IsConvertible` via properties on `ItemMetadata`. This is useful as it means we can evaluate these once upfront, instead of evaluating them each time we evaluate a property. These lookups were pretty cheap—they primarily involve a `Contains()` check on a `List<Type>`—but needing to do this at least once for every property during `MapAsync()` adds some overhead. Since we already have a cache of `ItemMetadata` via `TypeAccessor` (and the `TypeAccessorCache`), it's easy to include that in the cached metadata.
1 parent 63d32a2 commit d0fb8a4

2 files changed

Lines changed: 34 additions & 44 deletions

File tree

OnTopic/Internal/Reflection/ItemMetadata.cs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,19 @@ internal abstract class ItemMetadata {
2323
private readonly static List<Type> _listTypes = new();
2424
private readonly ICustomAttributeProvider _attributeProvider;
2525
private readonly Type _type = default!;
26+
private List<Attribute> _customAttributes = default!;
27+
28+
/*==========================================================================================================================
29+
| CONSTRUCTOR
30+
\-------------------------------------------------------------------------------------------------------------------------*/
31+
/// <summary>
32+
/// Establishes a new instance of a <see cref="ItemMetadata"/> with required dependencies.
33+
/// </summary>
34+
static ItemMetadata() {
35+
_listTypes.Add(typeof(IEnumerable<>));
36+
_listTypes.Add(typeof(ICollection<>));
37+
_listTypes.Add(typeof(IList<>));
38+
}
2639

2740
/*==========================================================================================================================
2841
| CONSTRUCTOR
@@ -101,6 +114,23 @@ bool isList()
101114
/// </remarks>
102115
internal bool IsNullable { get; init; }
103116

117+
/*==========================================================================================================================
118+
| IS LIST?
119+
\-------------------------------------------------------------------------------------------------------------------------*/
120+
/// <summary>
121+
/// Determine if the member is a <see cref="IList"/>, <see cref="IList{T}"/>, <see cref="IEnumerable{T}"/>, or <see cref="
122+
/// ICollection{T}"/>.
123+
/// </summary>
124+
internal bool IsList { get; init; }
125+
126+
/*==========================================================================================================================
127+
| IS CONVERTIBLE?
128+
\-------------------------------------------------------------------------------------------------------------------------*/
129+
/// <summary>
130+
/// Determine if the member is of a type that can be converted using the <see cref="AttributeValueConverter"/> class.
131+
/// </summary>
132+
internal bool IsConvertible { get; init; }
133+
104134
/*==========================================================================================================================
105135
| CUSTOM ATTRIBUTES
106136
\-------------------------------------------------------------------------------------------------------------------------*/

OnTopic/Mapping/TopicMappingService.cs

Lines changed: 4 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -30,19 +30,6 @@ public class TopicMappingService : ITopicMappingService {
3030
\-------------------------------------------------------------------------------------------------------------------------*/
3131
readonly ITopicRepository _topicRepository;
3232
readonly ITypeLookupService _typeLookupService;
33-
static readonly List<Type> _listTypes = new();
34-
35-
/*==========================================================================================================================
36-
| CONSTRUCTOR
37-
\-------------------------------------------------------------------------------------------------------------------------*/
38-
/// <summary>
39-
/// Establishes a new instance of a <see cref="TopicMappingService"/> with required dependencies.
40-
/// </summary>
41-
static TopicMappingService() {
42-
_listTypes.Add(typeof(IEnumerable<>));
43-
_listTypes.Add(typeof(ICollection<>));
44-
_listTypes.Add(typeof(IList<>));
45-
}
4633

4734
/*==========================================================================================================================
4835
| CONSTRUCTOR
@@ -404,7 +391,7 @@ private async Task<object> MapAsync(
404391
\-----------------------------------------------------------------------------------------------------------------------*/
405392
var value = await GetValue(source, parameter.Type, associations, configuration, cache, false).ConfigureAwait(false);
406393

407-
if (value is null && IsList(parameter.Type)) {
394+
if (value is null && parameter.IsList) {
408395
return await getList(parameter.Type, configuration).ConfigureAwait(false);
409396
}
410397

@@ -487,7 +474,7 @@ await MapAsync(
487474
\-----------------------------------------------------------------------------------------------------------------------*/
488475
else {
489476
var value = await GetValue(source, propertyAccessor.Type, associations, configuration, cache, mapAssociationsOnly).ConfigureAwait(false);
490-
if (value is null && IsList(propertyAccessor.Type)) {
477+
if (value is null && propertyAccessor.IsList) {
491478
await SetCollectionValueAsync(source, target, associations, configuration, cache).ConfigureAwait(false);
492479
}
493480
else if (value != null && propertyAccessor.CanWrite) {
@@ -545,12 +532,12 @@ await MapAsync(
545532
value = await GetTopicReferenceAsync(source.Parent, targetType, configuration, cache).ConfigureAwait(false);
546533
}
547534
}
548-
else if (AttributeValueConverter.IsConvertible(targetType)) {
535+
else if (configuration.Metadata.IsConvertible) {
549536
if (!mapAssociationsOnly) {
550537
value = GetScalarValue(source, configuration);
551538
}
552539
}
553-
else if (IsList(targetType)) {
540+
else if (configuration.Metadata.IsList) {
554541
return null;
555542
}
556543
else if (associations.HasFlag(AssociationTypes.References)) {
@@ -641,33 +628,6 @@ await MapAsync(
641628

642629
}
643630

644-
/*==========================================================================================================================
645-
| PRIVATE: IS LIST?
646-
\-------------------------------------------------------------------------------------------------------------------------*/
647-
/// <summary>
648-
/// Given a type, determines whether it's a list that is recognized by the <see cref="TopicMappingService"/>.
649-
/// </summary>
650-
/// <remarks>
651-
/// <para>
652-
/// To qualify, the <paramref name="targetType"/> must either implement <see cref="IList"/>, or it must be of type <see
653-
/// cref="IEnumerable{T}"/>, <see cref="ICollection{T}"/>, or <see cref="IList{T}"/>—any of which, if null, will be
654-
/// instantiated as a new <see cref="List{T}"/>.
655-
/// </para>
656-
/// <para>
657-
/// It is technically possible for the <paramref name="targetType"/> to implement one of the interfaces, such as <see
658-
/// cref="IList{T}"/>, while the assigned reference type is not compatible with the <see cref="IList"/> interface
659-
/// required by e.g. <see cref="PopulateTargetCollectionAsync(IList{Topic}, IList, ItemConfiguration, MappedTopicCache)"
660-
/// />. Detecting this requires looping through the interface implementations which is comparatively more costly given
661-
/// the number of times <see cref="IsList(Type)"/> gets called. In practice, collections that implement e.g. <see cref="
662-
/// IList{T}"/> are expected to also support <see cref="IList"/>. If they don't, however, the mapping will throw an
663-
/// exception since the assigned value will not be castable to an <see cref="IList"/>.
664-
/// </para>
665-
/// </remarks>
666-
/// <param name="targetType">The <see cref="Type"/> of collection to initialize.</param>
667-
private static bool IsList(Type targetType) =>
668-
typeof(IList).IsAssignableFrom(targetType) ||
669-
targetType.IsGenericType && _listTypes.Contains(targetType.GetGenericTypeDefinition());
670-
671631
/*==========================================================================================================================
672632
| PRIVATE: INITIALIZE COLLECTION
673633
\-------------------------------------------------------------------------------------------------------------------------*/

0 commit comments

Comments
 (0)