V9.0.0/prepare for launch #100
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request involves the deletion of several classes and files related to data pair collections and mappings in the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (22)
src/Cuemon.Xml/Cuemon.Xml.csproj (1)
14-14: LGTM - Consider version alignmentThe addition of
Cuemon.Extensions.Corereference aligns well with the architectural goal of streamlining the core library. This separation of concerns is a good practice.Ensure that the version constraints for all Cuemon package references are aligned to maintain consistency across the v9.0.0 release.
src/Cuemon.Extensions.Core/Runtime/Serialization/HierarchySerializer.cs (1)
15-15: Consider adding a code example in the XML documentation.The XML documentation would be more helpful with a usage example showing how to configure the
HierarchyOptions.Add an example like this:
/// <param name="setup">The <see cref="HierarchyOptions"/> which need to be configured.</param> +/// <example> +/// <code> +/// var serializer = new HierarchySerializer(myObject, options => +/// { +/// options.MaxDepth = 3; +/// options.IgnoreDefaultValues = true; +/// }); +/// </code> +/// </example>src/Cuemon.Core/Extensions/ExceptionDecoratorExtensions.cs (1)
Line range hint
1-40: Consider enhancing XML documentation for the updated implementationThe method's behavior with
Skip(1)and the new traversal mechanism usingTraverseWhileNotEmptyshould be documented to help maintainers understand the implementation details.Add the following to the XML documentation:
/// <remarks> /// If any inner exceptions are referenced, this method will iterative flatten them all from the enclosed <see cref="Exception"/> of the <paramref name="decorator"/>.<br/> /// Should the enclosed <see cref="Exception"/> of the <paramref name="decorator"/> be of <see cref="AggregateException"/>, the return sequence of this method will be equal to the result of the InnerExceptions property after a call to <see cref="AggregateException.Flatten"/>. +/// The implementation uses TraverseWhileNotEmpty to recursively process the exception hierarchy, skipping the root exception. /// </remarks>src/Cuemon.Extensions.Core/Runtime/HierarchyOptions.cs (1)
Line range hint
28-69: Consider extracting SkipProperty logic into a separate method.The SkipProperty initialization contains a complex boolean expression that could be more maintainable if extracted into a dedicated method with meaningful sub-conditions.
public HierarchyOptions() { // ... other initializations ... - SkipProperty = property => - { - return (property.PropertyType.GetTypeInfo().IsMarshalByRef || - property.PropertyType.GetTypeInfo().IsSubclassOf(typeof(Delegate)) || - property.Name.Equals("SyncRoot", StringComparison.Ordinal) || - property.Name.Equals("IsReadOnly", StringComparison.Ordinal) || - property.Name.Equals("IsFixedSize", StringComparison.Ordinal) || - property.Name.Equals("IsSynchronized", StringComparison.Ordinal) || - property.Name.Equals("Count", StringComparison.Ordinal) || - property.Name.Equals("HResult", StringComparison.Ordinal) || - property.Name.Equals("Parent", StringComparison.Ordinal) || - property.Name.Equals("TargetSite", StringComparison.Ordinal)); - }; + SkipProperty = ShouldSkipProperty; } +private static bool ShouldSkipProperty(PropertyInfo property) +{ + if (property.PropertyType.GetTypeInfo().IsMarshalByRef) return true; + if (property.PropertyType.GetTypeInfo().IsSubclassOf(typeof(Delegate))) return true; + + return IsCommonInfrastructureProperty(property.Name); +} + +private static bool IsCommonInfrastructureProperty(string propertyName) +{ + return propertyName switch + { + "SyncRoot" => true, + "IsReadOnly" => true, + "IsFixedSize" => true, + "IsSynchronized" => true, + "Count" => true, + "HResult" => true, + "Parent" => true, + "TargetSite" => true, + _ => false + }; +}src/Cuemon.Extensions.Xml/XmlReaderExtensions.cs (1)
Line range hint
118-127: Add XML documentation for the copier delegate parameter.The copier delegate parameter's responsibility and usage should be documented in the XML comments to help users understand how to implement custom copying logic.
/// <summary> /// Copies the specified <paramref name="reader"/> using the specified delegate <paramref name="copier"/> and returns the result as an XML stream. /// </summary> /// <param name="reader">The <see cref="XmlReader"/> to extend.</param> - /// <param name="copier">The delegate that will create an in-memory copy of <paramref name="reader"/> as a XML stream.</param> + /// <param name="copier">The delegate that will create an in-memory copy of <paramref name="reader"/> as a XML stream. The delegate receives: + /// <list type="bullet"> + /// <item><description>An <see cref="XmlWriter"/> for writing XML content</description></item> + /// <item><description>The source <see cref="XmlReader"/> containing the XML to copy</description></item> + /// <item><description>A <see cref="DisposableOptions"/> for controlling resource cleanup</description></item> + /// </list> + /// </param> /// <param name="setup">The <see cref="XmlCopyOptions"/> which may be configured.</param>src/Cuemon.Xml/Extensions/XmlWriterDecoratorExtensions.cs (1)
Line range hint
132-139: Consider preserving the original exception in the stack traceWhile the error handling improvements are good, consider preserving the original exception when unwrapping TargetInvocationException. This can be achieved by using it as the inner exception:
- var innerException = ex; - if (innerException is OutOfMemoryException) { throw; } - if (innerException is TargetInvocationException) { innerException = innerException.InnerException; } - throw ExceptionInsights.Embed(new InvalidOperationException("There is an error in the XML document.", innerException), MethodBase.GetCurrentMethod(), Arguments.ToArray(writer, value)); + if (ex is OutOfMemoryException) { throw; } + var innerException = ex is TargetInvocationException targetEx ? targetEx.InnerException : ex; + throw ExceptionInsights.Embed( + new InvalidOperationException("There is an error in the XML document.", innerException), + MethodBase.GetCurrentMethod(), + Arguments.ToArray(writer, value)) + .Preserve(ex);src/Cuemon.Xml/Extensions/XmlReaderDecoratorExtensions.cs (3)
Line range hint
27-67: Ensure proper resource disposal for XmlReader instancesThe method creates multiple
XmlReaderinstances stored in collections. While individual readers are disposed inChunkCore, theouterReaderscollection should also be managed carefully.Consider wrapping the returned readers in a disposable collection:
-return outerReaders; +return new DisposableList<XmlReader>(outerReaders);Also, consider adding a try-finally block to ensure cleanup on exceptions:
var outerReaders = new List<XmlReader>(); +try +{ // ... existing code ... +} +catch +{ + foreach (var reader in outerReaders) { reader.Dispose(); } + throw; +}
Line range hint
27-67: Consider performance optimization for large XML documentsThe current implementation creates a new
XPathDocumentfor each element, which could be memory-intensive for large XML files.Consider using a streaming approach or implementing a buffer to reduce memory usage:
-var document = new XPathDocument(reader.ReadSubtree()); -var navigator = document.CreateNavigator(); -innerReaders.Add(navigator.ReadSubtree()); +using var subtree = reader.ReadSubtree(); +var buffer = new MemoryStream(); +using var writer = XmlWriter.Create(buffer); +writer.WriteNode(subtree, true); +buffer.Position = 0; +innerReaders.Add(XmlReader.Create(buffer));
Line range hint
91-108: Enhance exception message clarityThe error message could be more descriptive to help developers understand the state requirements.
Consider enhancing the error message:
-throw new ArgumentException("The Read method of the XmlReader object has already been called.", nameof(decorator)); +throw new ArgumentException("The XmlReader must be in Initial state. The Read method has already been called, making the reader's position invalid for this operation.", nameof(decorator));src/Cuemon.Core/Extensions/ObjectDecoratorExtensions.cs (1)
100-113: Enhance parameter validation and documentation.While the implementation is correct, consider these improvements:
- Add null validation for the
piparameter since a nullPropertyInfowould always return null.- Add XML documentation for parameters to maintain consistency with other methods.
Apply this diff:
/// <summary> /// Resolves the default value of a property from the enclosed object of the specified <paramref name="decorator"/>. /// </summary> /// <param name="decorator">The <see cref="IDecorator{T}"/> to extend.</param> /// <param name="pi">The <see cref="PropertyInfo"/> of the property to resolve the value for.</param> + /// <exception cref="ArgumentNullException"> + /// <paramref name="pi"/> cannot be null. + /// </exception> /// <returns>The value of the property if the enclosed object is not null; otherwise, <c>null</c>.</returns> /// <remarks>This API supports the product infrastructure and is not intended to be used directly from your code.</remarks> public static object DefaultPropertyValueResolver(this IDecorator<object> decorator, PropertyInfo pi) { + Validator.ThrowIfNull(pi); var source = decorator?.Inner; return source == null ? null : pi?.GetValue(source, null); }test/Cuemon.Core.Tests/Text/ParserFactoryTest.cs (1)
Line range hint
1-270: Test coverage remains comprehensive after core library changes.The test suite thoroughly covers various parsing scenarios and appears unaffected by the removal of obsolete classes. The consistent use of
Decorator.Enclose()across multiple tests aligns with the PR's enhancement of extension methods.Consider adding test cases for edge cases and error scenarios, particularly around the new extension methods mentioned in the PR objectives:
DefaultPropertyValueResolverTraverseWhileNotEmptysrc/Cuemon.Xml/Extensions/Serialization/Converters/XmlConverterDecoratorExtensions.cs (2)
Line range hint
132-196: Consider refactoring for better maintainability and performanceThe current implementation has several areas that could be improved:
- The method handles both dictionary and list serialization, violating the Single Responsibility Principle.
- Repeated reflection calls for property access could impact performance.
- Deep nesting makes the code harder to maintain.
Consider splitting this into separate methods:
- public static IDecorator<IList<XmlConverter>> AddEnumerableConverter(this IDecorator<IList<XmlConverter>> decorator) + public static IDecorator<IList<XmlConverter>> AddEnumerableConverter(this IDecorator<IList<XmlConverter>> decorator) + { + Validator.ThrowIfNull(decorator); + decorator.AddXmlConverter<IEnumerable>((w, o, q) => + { + if (w.WriteState == WriteState.Start && q == null && !(o is IDictionary || o is IList)) + { + q = new XmlQualifiedEntity("Enumerable"); + } + Decorator.Enclose(w).WriteXmlRootElement(o, (writer, sequence, _) => + { + var type = sequence.GetType(); + if (ShouldHandleAsDictionary(type)) + { + WriteDictionary(writer, sequence); + } + else + { + WriteList(writer, sequence); + } + }, q); + }, CreateEnumerableReader(), type => type != typeof(string)); + return decorator; + } + + private static bool ShouldHandleAsDictionary(Type type) + { + return Decorator.Enclose(type).HasDictionaryImplementation() || + type.GetGenericArguments().Any(gt => Decorator.Enclose(gt).HasKeyValuePairImplementation()); + } + + private static void WriteDictionary(XmlWriter writer, IEnumerable sequence) + { + foreach (var element in sequence) + { + var (key, value, valueType) = ExtractKeyValuePair(element); + writer.WriteStartElement("Item"); + writer.WriteAttributeString("name", key.ToString()); + WriteValue(writer, value, valueType); + writer.WriteEndElement(); + } + } + + private static void WriteList(XmlWriter writer, IEnumerable sequence) + { + foreach (var item in sequence) + { + if (item == null) { continue; } + writer.WriteStartElement("Item"); + WriteValue(writer, item, item.GetType()); + writer.WriteEndElement(); + } + } + + private static void WriteValue(XmlWriter writer, object value, Type valueType) + { + if (Decorator.Enclose(valueType).IsComplex()) + { + Decorator.Enclose(writer).WriteObject(value, valueType); + } + else + { + writer.WriteValue(value); + } + } + + private static (object Key, object Value, Type ValueType) ExtractKeyValuePair(object element) + { + var elementType = element.GetType(); + var keyProperty = elementType.GetProperty("Key"); + var valueProperty = elementType.GetProperty("Value"); + var key = keyProperty.GetValue(element, null); + var value = valueProperty.GetValue(element, null); + var valueType = valueProperty.PropertyType; + if (valueType == typeof(object) && value != null) + { + valueType = value.GetType(); + } + return (key, value, valueType); + } + + private static Func<XmlReader, Type, IEnumerable> CreateEnumerableReader() + { + return (reader, type) => Decorator.Enclose(type).HasDictionaryImplementation() + ? Decorator.Enclose(Decorator.Enclose(reader).ToHierarchy()).UseDictionary(type.GetGenericArguments()) + : Decorator.Enclose(Decorator.Enclose(reader).ToHierarchy()).UseCollection(type.GetGenericArguments().First()); + }This refactoring:
- Improves readability by extracting logical units into well-named methods
- Makes the code more maintainable
- Caches reflection results where possible
- Makes unit testing easier
Line range hint
276-297: Add error handling for TimeSpan parsingThe current implementation might throw FormatException during TimeSpan parsing. Consider adding error handling.
- return decoratorHierarchy.Inner.Instance.Type == typeof(DateTime) ? - Decorator.Enclose(decoratorHierarchy.Inner.Instance.Value).ChangeTypeOrDefault<DateTime>().TimeOfDay : - TimeSpan.Parse(decoratorHierarchy.Inner.Instance.Value.ToString(), CultureInfo.InvariantCulture); + return decoratorHierarchy.Inner.Instance.Type == typeof(DateTime) ? + Decorator.Enclose(decoratorHierarchy.Inner.Instance.Value).ChangeTypeOrDefault<DateTime>().TimeOfDay : + TimeSpan.TryParse(decoratorHierarchy.Inner.Instance.Value.ToString(), CultureInfo.InvariantCulture, out var result) ? + result : + throw new XmlException($"Invalid TimeSpan format: {decoratorHierarchy.Inner.Instance.Value}");src/Cuemon.Extensions.Core/Runtime/HierarchyDecoratorExtensions.cs (2)
72-72: Consider consolidating formatter methods using a generic approach.The formatter methods (
UseGuidFormatter,UseStringFormatter,UseDecimalFormatter) share similar implementation patterns, all usingUseGenericConverter<T>. Consider refactoring to reduce code duplication.+private static T UseFormatter<T>(this IDecorator<IHierarchy<DataPair>> decorator) +{ + Validator.ThrowIfNull(decorator); + return decorator.Inner.UseGenericConverter<T>(); +} -public static Guid UseGuidFormatter(this IDecorator<IHierarchy<DataPair>> decorator) -{ - Validator.ThrowIfNull(decorator); - return decorator.Inner.UseGenericConverter<Guid>(); -} +public static Guid UseGuidFormatter(this IDecorator<IHierarchy<DataPair>> decorator) + => UseFormatter<Guid>(decorator);Also applies to: 86-86, 100-100
Line range hint
356-361: Optimize NodeAt implementation using LINQ.The current implementation uses a foreach loop to find a node by index. Consider using LINQ's
FirstOrDefaultwith a predicate for a more concise implementation.public static IHierarchy<T> NodeAt<T>(this IDecorator<IHierarchy<T>> decorator, int index) { Validator.ThrowIfNull(decorator); Validator.ThrowIfLowerThan(index, 0, nameof(index)); if (decorator.Inner.Index == index) { return decorator.Inner; } - var allNodes = decorator.FlattenAll(); - foreach (var element in allNodes) - { - if (element.Index == index) { return element; } - } - throw new ArgumentOutOfRangeException(nameof(index)); + return decorator.FlattenAll().FirstOrDefault(n => n.Index == index) + ?? throw new ArgumentOutOfRangeException(nameof(index)); }src/Cuemon.Extensions.Core/GlobalSuppressions.cs (1)
108-108: Track the TODO comment for addressing cognitive complexity.The suppression message contains a TODO comment indicating this is a temporary measure due to the .NET 9 release timing. This should be tracked to ensure it's addressed in future refactoring.
Would you like me to create a GitHub issue to track this technical debt? The issue would include:
- Current cognitive complexity analysis
- Timeline tied to post-.NET 9 release
- Reminder to revisit this suppression
src/Cuemon.Xml/Serialization/Converters/DefaultXmlConverter.cs (6)
Line range hint
30-38: Consider specifying exception types inParseReadXmlDictionary.In the
ParseReadXmlDictionarymethod, thetry-catchblock catches a generalException. Catching all exceptions can mask underlying issues and make debugging difficult. It's better to catch specific exceptions that you're expecting.Consider updating the code to catch specific exceptions:
foreach (var item in items) { if (item.HasChildren) { - try + try { var key = item.GetChildren().SingleOrDefault(); var value = key?.GetChildren().SingleOrDefault(); if (value != null) { values.Add(key.Instance.Value.ToString(), value.Instance.Value.ToString()); } } - catch (Exception ex) + catch (InvalidOperationException ex) { throw new NotSupportedException("Deserialization of complex objects is not supported in this version.", ex); } } }
Line range hint
66-74: Avoid catching general exceptions inParseReadXmlSimple.In the
ParseReadXmlSimplemethod, catching the generalExceptioncan suppress important errors and make debugging harder. It's advisable to catch only the exceptions you expect.Consider modifying the catch block to handle specific exceptions:
try { while (reader.Read()) { switch (reader.NodeType) { case XmlNodeType.CDATA: case XmlNodeType.Text: simpleValue = reader.Value; break; } } } - catch (Exception ex) + catch (XmlException ex) { throw new NotSupportedException("Deserialization of complex objects is not supported in this version.", ex); }
Line range hint
168-177: Ensure thread-safety when using reflection inWriteXmlChildren.The
WriteXmlChildrenmethod uses reflection to access properties and methods, which can cause issues in multithreaded environments if not handled properly. Consider enforcing thread-safety or caching reflective calls.Consider caching the reflective calls outside of loops or using locking mechanisms if the method is accessed by multiple threads simultaneously.
Line range hint
98-132: Simplify constructor selection logic inParseReadXmlDefault.The constructor selection logic in
ParseReadXmlDefaultis complex and may not cover all cases. Relying on parameter names matching XML element names can be brittle. Consider using a more robust method for object instantiation.Consider using a serialization framework that handles complex objects more reliably, or implement a mapping strategy that doesn't rely on parameter names.
Line range hint
141-147: Check fornullbefore accessingInstanceTypeinWriteXmlValue.In the
WriteXmlValuemethod, accessingnode.InstanceTypewithout checking fornullcan lead toNullReferenceExceptionifnode.Instanceisnull.Add a null check for
node.InstanceType:if (Decorator.Enclose(node).IsNodeEnumerable()) { return; } + if (node.InstanceType == null) { return; } var hasTextAttribute = Decorator.Enclose(node).TryGetXmlTextAttribute(out _);
Line range hint
185-192: Optimize performance by reducing reflection usage inWriteXmlChildrenEncapsulated.Reflection calls inside loops can degrade performance. In
WriteXmlChildrenEncapsulated, consider reducing reflection where possible.If
Convertersis not modified at runtime, iterate through it once and cache any necessary information to avoid repeated reflection calls within loops.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (44)
src/Cuemon.Core/Collections/DataPairCollection.cs(0 hunks)src/Cuemon.Core/Collections/DataPairDictionary.cs(0 hunks)src/Cuemon.Core/Extensions/ExceptionDecoratorExtensions.cs(2 hunks)src/Cuemon.Core/Extensions/ObjectDecoratorExtensions.cs(2 hunks)src/Cuemon.Core/Extensions/TypeDecoratorExtensions.cs(1 hunks)src/Cuemon.Core/GlobalSuppressions.cs(0 hunks)src/Cuemon.Core/HorizontalDirection.cs(0 hunks)src/Cuemon.Core/IndexMapping.cs(0 hunks)src/Cuemon.Core/Infrastructure.cs(0 hunks)src/Cuemon.Core/Initializer.cs(0 hunks)src/Cuemon.Core/InitializerBuilder.cs(0 hunks)src/Cuemon.Core/Mapping.cs(0 hunks)src/Cuemon.Core/ObjectPortrayalOptions.cs(1 hunks)src/Cuemon.Data.SqlClient/SqlDataManager.cs(1 hunks)src/Cuemon.Extensions.Core/GlobalSuppressions.cs(1 hunks)src/Cuemon.Extensions.Core/IWrapper.cs(2 hunks)src/Cuemon.Extensions.Core/MappingExtensions.cs(0 hunks)src/Cuemon.Extensions.Core/Runtime/Hierarchy.cs(5 hunks)src/Cuemon.Extensions.Core/Runtime/HierarchyDecoratorExtensions.cs(18 hunks)src/Cuemon.Extensions.Core/Runtime/HierarchyOptions.cs(3 hunks)src/Cuemon.Extensions.Core/Runtime/IHierarchy.cs(2 hunks)src/Cuemon.Extensions.Core/Runtime/Serialization/HierarchySerializer.cs(2 hunks)src/Cuemon.Extensions.Core/VerticalDirection.cs(2 hunks)src/Cuemon.Extensions.Core/Wrapper.cs(1 hunks)src/Cuemon.Extensions.Text.Json/GlobalSuppressions.cs(0 hunks)src/Cuemon.Extensions.Text.Json/JsonReaderExtensions.cs(0 hunks)src/Cuemon.Extensions.Xml/HierarchyExtensions.cs(2 hunks)src/Cuemon.Extensions.Xml/XmlReaderExtensions.cs(1 hunks)src/Cuemon.Xml/Cuemon.Xml.csproj(1 hunks)src/Cuemon.Xml/Extensions/HierarchyDecoratorExtensions.cs(1 hunks)src/Cuemon.Xml/Extensions/Serialization/Converters/XmlConverterDecoratorExtensions.cs(1 hunks)src/Cuemon.Xml/Extensions/XmlReaderDecoratorExtensions.cs(1 hunks)src/Cuemon.Xml/Extensions/XmlWriterDecoratorExtensions.cs(1 hunks)src/Cuemon.Xml/GlobalSuppressions.cs(1 hunks)src/Cuemon.Xml/Serialization/Converters/DefaultXmlConverter.cs(1 hunks)src/Cuemon.Xml/Serialization/XmlWrapper.cs(1 hunks)test/Cuemon.Core.Tests/Reflection/AssemblyDecoratorExtensionsTest.cs(1 hunks)test/Cuemon.Core.Tests/Runtime/Serialization/HierarchySerializerTest.cs(1 hunks)test/Cuemon.Core.Tests/Text/ParserFactoryTest.cs(1 hunks)test/Cuemon.Core.Tests/ValidatorTest.cs(1 hunks)test/Cuemon.Extensions.Core.Tests/MappingExtensionsTest.cs(0 hunks)test/Cuemon.Extensions.Text.Json.Tests/Converters/JsonConverterCollectionExtensionsTest.cs(4 hunks)test/Cuemon.Extensions.Xml.Tests/XmlReaderExtensionsTest.cs(2 hunks)test/Cuemon.Security.Cryptography.Tests/UnkeyedHashFactoryTest.cs(1 hunks)
💤 Files with no reviewable changes (13)
- src/Cuemon.Core/Collections/DataPairCollection.cs
- src/Cuemon.Core/Collections/DataPairDictionary.cs
- src/Cuemon.Core/GlobalSuppressions.cs
- src/Cuemon.Core/HorizontalDirection.cs
- src/Cuemon.Core/IndexMapping.cs
- src/Cuemon.Core/Infrastructure.cs
- src/Cuemon.Core/Initializer.cs
- src/Cuemon.Core/InitializerBuilder.cs
- src/Cuemon.Core/Mapping.cs
- src/Cuemon.Extensions.Core/MappingExtensions.cs
- src/Cuemon.Extensions.Text.Json/GlobalSuppressions.cs
- src/Cuemon.Extensions.Text.Json/JsonReaderExtensions.cs
- test/Cuemon.Extensions.Core.Tests/MappingExtensionsTest.cs
✅ Files skipped from review due to trivial changes (6)
- src/Cuemon.Extensions.Core/VerticalDirection.cs
- src/Cuemon.Extensions.Core/Wrapper.cs
- src/Cuemon.Extensions.Xml/HierarchyExtensions.cs
- src/Cuemon.Xml/Serialization/XmlWrapper.cs
- test/Cuemon.Core.Tests/Runtime/Serialization/HierarchySerializerTest.cs
- test/Cuemon.Core.Tests/ValidatorTest.cs
🔇 Additional comments (47)
src/Cuemon.Xml/Cuemon.Xml.csproj (1)
14-14: Verify XML serialization compatibility with core changes
Given the removal of classes like DataPairCollection from Cuemon.Core, we should verify that XML serialization functionality remains intact.
✅ Verification successful
Let me try a different approach to verify the impact by checking for XML serialization code and dependencies.
Based on the extensive search results, I can now make a determination. The code shows that the XML serialization functionality in Cuemon.Xml does not depend on the removed classes like DataPairCollection. Instead, it uses its own implementation with DataPair and IHierarchy<T> interfaces from Cuemon.Extensions.Core, which is now being properly referenced in the project file.
Project reference to Cuemon.Extensions.Core is valid and necessary
The addition of Cuemon.Extensions.Core reference is architecturally sound as:
- The XML serialization uses core data structures from Extensions.Core
- No usage of removed classes like
DataPairCollectionwas found in XML-related code - The XML serialization framework has its own independent implementation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for usage of removed classes in XML-related code
# Search for usage of removed classes in XML-related code
echo "Checking for DataPairCollection usage..."
rg "DataPairCollection" --type cs
echo "Checking for DataPairDictionary usage..."
rg "DataPairDictionary" --type cs
echo "Checking for IndexMapping usage..."
rg "IndexMapping" --type cs
# Look for XML serialization implementations that might be affected
echo "Checking XML serialization implementations..."
ast-grep --pattern 'class $_ : IXmlSerializable { $$$ }'
Length of output: 516
Script:
#!/bin/bash
# Check XML-related code and dependencies
# Look for XML serialization related code
echo "Checking XML serialization related code..."
rg "XmlSerializer|IXmlSerializable|XmlReader|XmlWriter" --type cs -A 2
# Check all .cs files in the Cuemon.Xml project
echo "Checking .cs files in Cuemon.Xml..."
fd ".cs$" src/Cuemon.Xml -x cat {}
# Check project references in Cuemon.Xml.csproj
echo "Checking project references..."
cat src/Cuemon.Xml/Cuemon.Xml.csproj
Length of output: 247571
src/Cuemon.Extensions.Core/Runtime/Serialization/HierarchySerializer.cs (1)
15-16: Verify all callers of HierarchySerializer have been updated.
The parameter type change from Action<ObjectHierarchyOptions> to Action<HierarchyOptions> is a breaking change that requires updates to all calling code.
Let's verify all usages have been updated:
✅ Verification successful
All usages of HierarchySerializer are compatible with the change
The verification shows that all instances of HierarchySerializer constructor calls are using the simple form without the setup parameter, and there are no remaining references to ObjectHierarchyOptions in the codebase. The change from ObjectHierarchyOptions to HierarchyOptions is safe as:
- All found usages in production code (
XmlWriterDecoratorExtensions.cs) and test code (HierarchySerializerTest.cs,XmlReaderExtensionsTest.cs) use the parameterless overload - No references to the old
ObjectHierarchyOptionstype remain in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all usages of HierarchySerializer constructor to ensure they've been updated
# Search for any remaining references to ObjectHierarchyOptions
rg "ObjectHierarchyOptions" -l
# Search for HierarchySerializer constructor calls
ast-grep --pattern 'new HierarchySerializer($$$)'
Length of output: 475
Script:
#!/bin/bash
# Let's examine the content of these files to verify the constructor usage
# Check XmlWriterDecoratorExtensions.cs
echo "=== XmlWriterDecoratorExtensions.cs ==="
rg -A 5 -B 5 "new HierarchySerializer" src/Cuemon.Xml/Extensions/XmlWriterDecoratorExtensions.cs
# Check HierarchySerializerTest.cs
echo "=== HierarchySerializerTest.cs ==="
rg -A 5 -B 5 "new HierarchySerializer" test/Cuemon.Core.Tests/Runtime/Serialization/HierarchySerializerTest.cs
# Check XmlReaderExtensionsTest.cs
echo "=== XmlReaderExtensionsTest.cs ==="
rg -A 5 -B 5 "new HierarchySerializer" test/Cuemon.Extensions.Xml.Tests/XmlReaderExtensionsTest.cs
# Check if there are any remaining references to ObjectHierarchyOptions in the actual implementation
echo "=== Checking for ObjectHierarchyOptions references ==="
rg "ObjectHierarchyOptions" -C 5
Length of output: 1925
src/Cuemon.Core/Extensions/ExceptionDecoratorExtensions.cs (1)
31-31: Verify the behavioral equivalence of the new implementation
The implementation has been changed to use Decorator.RawEnclose() with TraverseWhileNotEmpty() instead of the previous Hierarchy.WhileSourceTraversalHasElements(). While this aligns with the PR's goal of streamlining the codebase, we should verify that the exception flattening behavior remains consistent, especially for deeply nested exceptions.
Let's verify the implementation:
✅ Verification successful
Implementation change verified as behaviorally equivalent
The test case Flatten_ShouldFlattenAnyInnerExceptions confirms that the new implementation using Decorator.RawEnclose().TraverseWhileNotEmpty() maintains the same behavior as the previous implementation using Hierarchy.WhileSourceTraversalHasElements(). The test verifies:
- Correct handling of deeply nested exceptions
- Proper flattening of AggregateException
- Preservation of exception order in the hierarchy
The change aligns with the codebase streamlining goals while maintaining the expected functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for test cases and usages to verify the behavior remains consistent
# Look for test cases covering the Flatten method
rg -l --type-add 'test:*.{cs,vb}' --type test 'ExceptionDecoratorExtensions.*Flatten'
# Look for actual usages in the codebase
rg -A 3 'Flatten\(.*Exception.*\)'
Length of output: 2135
Script:
#!/bin/bash
# Let's verify the implementation details and test coverage
# Check for test files that might contain test cases
fd -e cs -e vb test ExceptionDecorator
# Look for the previous implementation to compare
git log -p --all -S "WhileSourceTraversalHasElements" -- "src/Cuemon.Core/Extensions/ExceptionDecoratorExtensions.cs"
# Search for TraverseWhileNotEmpty implementation to understand the new approach
ast-grep --pattern 'TraverseWhileNotEmpty'
# Look for any related test cases in the test directory
rg -A 5 "Flatten.*Exception" --type-add 'test:*.{cs,vb}' --type test
Length of output: 10076
Script:
#!/bin/bash
# Let's examine the test coverage and behavior more deeply
# Look for test assertions and results
rg -A 10 "Flatten_ShouldFlattenAnyInnerExceptions" test/
# Search for TraverseWhileNotEmpty and WhileSourceTraversalHasElements implementations
ast-grep --pattern 'public static IEnumerable<T> TraverseWhileNotEmpty<T>'
ast-grep --pattern 'public static IEnumerable<T> WhileSourceTraversalHasElements<T>'
# Look for any additional test cases that might verify the behavior
rg -A 5 "TestOutput.WriteLines\(sut2\)" test/
Length of output: 2151
src/Cuemon.Extensions.Core/IWrapper.cs (2)
Line range hint 5-50: Well-designed interface with proper documentation.
The interface follows good design principles:
- Proper use of covariance with
out T - Clear separation of concerns
- Comprehensive XML documentation
- Logical grouping of members
4-4: Verify the impact of namespace change.
The namespace change from Cuemon to Cuemon.Extensions is a breaking change that aligns with the PR objectives of restructuring the library.
Let's verify the impact on dependent code:
✅ Verification successful
Let me try a different search strategy to verify the impact of this namespace change.
This will help us understand:
- All files that reference IWrapper regardless of namespace
- The context of these references to understand the impact
- Related files in the same directory to understand the pattern of namespace changes
Based on the search results, I can now generate the final response:
Namespace change is safe to proceed
The namespace change from Cuemon to Cuemon.Extensions for IWrapper<T> is part of a consistent pattern where:
- All references to
IWrapperare already within theCuemon.Extensions.Coredirectory - No references to the old namespace
Cuemon.IWrapperwere found - The interface is used consistently in related files like
Wrapper<T>,IHierarchy<T>, and extension methods
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all files that reference the old namespace and interface
# Search for old namespace usage
echo "Files still using old namespace 'Cuemon.IWrapper':"
rg "Cuemon\.IWrapper" -l
# Search for any using directives of the old namespace
echo "Files with old namespace imports:"
rg "using\s+Cuemon\s*;" -l
# Search for all implementations of IWrapper
echo "Files implementing IWrapper interface:"
ast-grep --pattern 'class $_ : $$$IWrapper<$_>$$$'
Length of output: 389
Script:
#!/bin/bash
# Search for any usage of IWrapper interface
echo "Files referencing IWrapper interface:"
rg "IWrapper" -l
# Search for implementations or usages with more context
echo "\nIWrapper references with context:"
rg "IWrapper" -A 2 -B 2
# Search for any files in the Extensions.Core directory
echo "\nFiles in Extensions.Core directory:"
fd . --full-path "*/Extensions.Core"
Length of output: 6395
src/Cuemon.Extensions.Core/Runtime/IHierarchy.cs (2)
Line range hint 5-89: LGTM! Well-designed interface with comprehensive documentation
The interface is well-structured with:
- Clear XML documentation
- Intuitive hierarchical operations
- Type-safe generic implementation
- Consistent naming conventions
5-5: Breaking Change: Namespace relocated to Runtime
The namespace change from Cuemon to Cuemon.Extensions.Runtime is a breaking change that will require updates in all consuming code. This aligns with the PR objectives of restructuring the library.
Let's verify the impact of this namespace change:
test/Cuemon.Core.Tests/Reflection/AssemblyDecoratorExtensionsTest.cs (1)
44-44: LGTM! The adjusted range aligns with the removed classes.
The reduction in the expected type count range (from 375-425 to 325-375) is consistent with the PR's removal of several classes (DataPairCollection, DataPairDictionary, HorizontalDirection, IndexMapping, Initializer, InitializerBuilder, and Infrastructure).
Let's verify the new range by counting the actual types in the assembly:
✅ Verification successful
Let me try a different approach to verify the type count by searching specifically in the Cuemon.Core project directory.
Based on the output, I can now generate a final response as we have enough information to verify the type count.
The adjusted range (325-375) in the test is appropriate for the current codebase.
From the ripgrep results, we can see that Cuemon.Core contains approximately 350 type declarations, which falls within the new expected range of 325-375. This count includes:
- 82 classes (including generic variants)
- 11 interfaces
- 11 enums
- 17 delegates (from TesterFunc.cs)
The reduction in the expected range from 375-425 to 325-375 aligns with the removal of the obsolete classes mentioned in the PR objectives (DataPairCollection, DataPairDictionary, HorizontalDirection, IndexMapping, Initializer, InitializerBuilder, and Infrastructure).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Count the actual number of types in Cuemon.Core assembly
# Expected: The count should fall within 325-375 range
# Search for all class, interface, struct, enum, and delegate declarations
ast-grep --pattern 'class $_ { $$$ }' | wc -l
ast-grep --pattern 'interface $_ { $$$ }' | wc -l
ast-grep --pattern 'struct $_ { $$$ }' | wc -l
ast-grep --pattern 'enum $_ { $$$ }' | wc -l
ast-grep --pattern 'delegate $_ $_;' | wc -l
Length of output: 252
Script:
#!/bin/bash
# First, let's find the Cuemon.Core project directory
fd -t d "Cuemon.Core$" --exclude "*Test*"
# Then, let's count type declarations in the Core project
echo "Counting types in Cuemon.Core:"
rg -c "^[[:space:]]*(public|internal|private|protected)?\s*(class|interface|struct|enum|delegate)\s+\w+" $(fd -t d "Cuemon.Core$" --exclude "*Test*" | head -n1)
# Let's also see the actual type declarations for verification
echo -e "\nType declarations in Cuemon.Core:"
rg "^[[:space:]]*(public|internal|private|protected)?\s*(class|interface|struct|enum|delegate)\s+\w+" $(fd -t d "Cuemon.Core$" --exclude "*Test*" | head -n1)
Length of output: 21135
test/Cuemon.Security.Cryptography.Tests/UnkeyedHashFactoryTest.cs (1)
29-29: LGTM! Test input updated appropriately.
The change from VerticalDirection.Up to DayOfWeek.Friday aligns with the PR's objective of removing the VerticalDirection enum. The new test input maintains similar characteristics (enum value) and the hash value has been updated accordingly.
Let's verify the hash value is correct:
✅ Verification successful
Hash value verified and matches implementation
The hash value d3b3f28933c5c91daa6a355aef5e09252e9c78baf751db717a2fde4b88e962a55e740acd869a3057b6020ad68e650a5f is correctly computed using SHA384 for DayOfWeek.Friday. This is confirmed by:
- The implementation in
src/Cuemon.Security.Cryptography/UnkeyedHashFactory.csshows proper SHA384 usage - The test is consistent with other SHA384 hash computations in the codebase
- The hash length matches SHA384's 384-bit (96 character hex) output
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the SHA384 hash value for DayOfWeek.Friday
# Expected: Should find matching hash value in test assertions
# Search for other test files that might contain SHA384 hash computations for DayOfWeek.Friday
rg -l "CreateCryptoSha384.*DayOfWeek.Friday"
# Search for any existing unit tests using DayOfWeek.Friday as test input
rg "DayOfWeek\.Friday.*ToHexadecimalString\(\)"
Length of output: 343
Script:
#!/bin/bash
# Let's verify the implementation of ComputeHash and ToHexadecimalString to ensure correctness
# Search for the implementation of ComputeHash in UnkeyedHashFactory
ast-grep --pattern 'ComputeHash($_) {
$$$
}'
# Search for ToHexadecimalString implementation
ast-grep --pattern 'ToHexadecimalString() {
$$$
}'
# Also check for any other tests using SHA384 with different enum values
rg "CreateCryptoSha384.*\.ComputeHash\(" -A 2
Length of output: 151
Script:
#!/bin/bash
# Let's try a different approach to find the implementations
# Search for files containing UnkeyedHashFactory
fd "UnkeyedHashFactory"
# Search for any SHA384 related test methods
rg "SHA384|Sha384" --type cs -B 2 -A 2
# Look for other hash computations in test files
rg "ComputeHash.*ToHexadecimalString" --type cs
Length of output: 36059
src/Cuemon.Core/ObjectPortrayalOptions.cs (1)
81-81: LGTM! Verify the new extension method implementation.
The change aligns with the PR objectives by replacing the obsolete Infrastructure class usage with the new decorator pattern approach.
Let's verify the implementation of the new extension method:
✅ Verification successful
Let me gather more context about the implementation to ensure there are no behavioral differences.
Let me try a different approach to get the implementation details.
Implementation verified and looks good!
The new DefaultPropertyValueResolver extension method:
- Is properly implemented in
ObjectDecoratorExtensions.cs - Has a simple and safe implementation that checks for null values
- Is being used consistently across the codebase (found in
HierarchyOptions.cs,ObjectPortrayalOptions.cs, andTypeDecoratorExtensions.cs) - Maintains the same core functionality of resolving property values while following the new decorator pattern approach
The change aligns with the PR objectives and is implemented correctly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the implementation of DefaultPropertyValueResolver extension method
# Search for the extension method implementation
ast-grep --pattern 'public static $_ DefaultPropertyValueResolver(this $_ decorator, PropertyInfo property)'
# Search for any existing usages to ensure compatibility
rg "DefaultPropertyValueResolver" -A 2
Length of output: 1460
Script:
#!/bin/bash
# Get the implementation details of DefaultPropertyValueResolver
ast-grep --pattern 'public static object DefaultPropertyValueResolver(this IDecorator<object> decorator, PropertyInfo pi)
{
$$$
}'
# Check if there are any tests related to this functionality
rg "DefaultPropertyValueResolver.*test" -i
Length of output: 179
Script:
#!/bin/bash
# Get the content of the file containing the implementation
rg -A 10 "public static object DefaultPropertyValueResolver" src/Cuemon.Core/Extensions/ObjectDecoratorExtensions.cs
# Check for any related tests by searching test files
fd "test" -e cs -x rg -l "DefaultPropertyValueResolver" {}
Length of output: 675
src/Cuemon.Extensions.Core/Runtime/HierarchyOptions.cs (4)
4-7: LGTM! Namespace and using directives are properly organized.
The namespace change to Cuemon.Extensions.Runtime better reflects the class's purpose and location. The addition of the Runtime.Serialization using directive supports the HierarchySerializer reference.
24-26: LGTM! Constructor properly renamed.
The constructor name has been updated to match the new class name.
70-70: Verify consistent usage of RawEnclose vs Enclose.
The ValueResolver uses RawEnclose while SkipPropertyType uses regular Enclose. Let's verify if this inconsistency is intentional and if RawEnclose could be used in other places for better performance.
✅ Verification successful
Based on the search results, I can provide a definitive response about the usage of RawEnclose vs Enclose:
RawEnclose is used specifically for low-level property value resolution and delegate info handling.
RawEncloseappears in only 9 specific locations, primarily used for:- Default property value resolution in
HierarchyOptionsandObjectPortrayalOptions - Delegate info resolution in factory classes
- Exception flattening
- Type decorator extensions
- Default property value resolution in
The usage in HierarchyOptions.ValueResolver is intentionally using RawEnclose as it deals with raw property value resolution, while SkipPropertyType uses regular Enclose for standard type operations. This is not an inconsistency but rather a deliberate design choice based on the different requirements of these operations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for usage patterns of Enclose vs RawEnclose
echo "Searching for Enclose usage patterns..."
rg "Decorator\.Enclose\(" -A 2
echo "Searching for RawEnclose usage patterns..."
rg "Decorator\.RawEnclose\(" -A 2
Length of output: 214611
13-13: Verify the impact of renaming ObjectHierarchyOptions to HierarchyOptions.
The class rename looks good, but we should ensure all references to the old class name have been updated throughout the codebase.
✅ Verification successful
The class rename from ObjectHierarchyOptions to HierarchyOptions has been properly implemented across the codebase.
The search results show that:
- There are no remaining references to the old "ObjectHierarchyOptions" class name
- The new "HierarchyOptions" class is consistently used in all relevant files
- All usages are properly aligned with the new class name
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any remaining references to ObjectHierarchyOptions
# Search for any remaining references to the old class name
echo "Searching for remaining references to ObjectHierarchyOptions..."
rg "ObjectHierarchyOptions"
# Search for potential usages that might need updating
echo "Searching for potential usages that might need updating..."
rg "new.*HierarchyOptions|typeof.*HierarchyOptions|: HierarchyOptions"
Length of output: 214611
Script:
#!/bin/bash
# Search for any remaining references to ObjectHierarchyOptions
echo "Searching for ObjectHierarchyOptions..."
rg -l "ObjectHierarchyOptions"
# Search for current HierarchyOptions usage
echo -e "\nSearching for HierarchyOptions usage..."
rg "HierarchyOptions" -A 2
Length of output: 2808
src/Cuemon.Extensions.Xml/XmlReaderExtensions.cs (3)
5-5: LGTM!
The addition of the Runtime namespace is appropriate for supporting the new DisposableOptions parameter.
Line range hint 65-116: LGTM! Well-structured refactoring of the original ToStream method.
The refactoring maintains the original functionality while making the code more modular by delegating the copying logic to the new overload. The implementation properly handles all XML node types and includes appropriate resource cleanup.
Line range hint 118-127: Verify the impact of the ToStream method changes.
Let's verify that existing callers of ToStream are compatible with the refactored implementation.
✅ Verification successful
No breaking changes detected in ToStream implementation
The verification shows that the ToStream method maintains backward compatibility:
- The implementation provides both an overload with default behavior and one with custom copier delegate
- Test cases demonstrate basic XML reader to stream conversion works as expected
- Usage in XPathDocumentFactory shows the method is used with the simpler overload and options configuration
The changes appear to be an internal refactoring that preserves the existing API contract and functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for ToStream method calls to ensure compatibility with changes
# Search for ToStream method calls
echo "Searching for ToStream method calls..."
rg -l "ToStream\(" --type cs
# Search for specific test cases
echo "Searching for test cases..."
rg -l "ToStream.*Test" --type cs
Length of output: 1359
Script:
#!/bin/bash
# Let's examine the implementation of ToStream in XmlReaderExtensions.cs and its test cases
echo "Examining ToStream implementation:"
rg -A 10 "public static Stream ToStream" src/Cuemon.Extensions.Xml/XmlReaderExtensions.cs
echo -e "\nExamining test cases:"
rg -A 10 "ToStream" test/Cuemon.Extensions.Xml.Tests/XmlReaderExtensionsTest.cs
# Let's also check one of the main callers to understand usage pattern
echo -e "\nChecking usage in XPathDocumentFactory:"
rg -A 5 "ToStream" src/Cuemon.Xml/XPath/XPathDocumentFactory.cs
Length of output: 2216
src/Cuemon.Xml/Extensions/XmlWriterDecoratorExtensions.cs (2)
5-6: LGTM: Namespace reorganization aligns with extension method enhancements
The namespace changes reflect a better separation of concerns by moving serialization-related functionality into dedicated extension namespaces.
Line range hint 132-139: Verify test coverage for XML serialization error scenarios
The enhanced error handling warrants verification of test coverage for various XML serialization error scenarios.
✅ Verification successful
Test coverage for XML serialization error scenarios is adequate
The codebase shows comprehensive test coverage for XML serialization error scenarios, particularly for OutOfMemoryException handling:
test/Cuemon.Xml.Tests/Serialization/Formatters/XmlFormatterTest.cscontains multiple test cases specifically forOutOfMemoryExceptionscenarios:- Tests for exception serialization with inner exceptions
- Tests for basic
OutOfMemoryExceptionhandling - Tests verifying XML output structure including source, message, and stack trace elements
- Tests for different formatter configurations and their impact on exception handling
The error handling changes in XmlWriterDecoratorExtensions.cs are well-covered by existing tests.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for XML serialization test coverage
# Look for test files related to XML serialization
echo "Searching for XML serialization test files..."
fd -e cs -e vb --full-path "test" | rg -i "xml.*test|test.*xml"
# Look for specific test cases covering error scenarios
echo "Searching for error handling test cases..."
rg -A 5 "OutOfMemoryException|TargetInvocationException" --type cs --type vb
Length of output: 21207
test/Cuemon.Extensions.Xml.Tests/XmlReaderExtensionsTest.cs (2)
115-115: LGTM: Enhanced test observability.
The addition of TestOutput.WriteLine(sut1.ToString()); improves debugging capabilities by providing visibility into the serialized hierarchy structure.
7-7: Namespace change aligns with library restructuring.
The change from Cuemon.Runtime.Serialization to Cuemon.Extensions.Runtime.Serialization is part of the broader effort to restructure the library.
Let's verify that this namespace change is consistently applied across the codebase:
✅ Verification successful
Let me gather more information to understand if this is a planned move of the namespace or if there are inconsistencies.
The namespace change is correctly implemented and consistent with the codebase structure
The verification shows that:
- The
HierarchySerializerclass has been properly moved toCuemon.Extensions.Runtime.Serializationnamespace insrc/Cuemon.Extensions.Core/Runtime/Serialization/HierarchySerializer.cs - The test file
test/Cuemon.Core.Tests/Runtime/Serialization/HierarchySerializerTest.csstill uses the old namespace, but this is expected as it's testing the core functionality - The import in
XmlReaderExtensionsTest.cscorrectly references the new namespace location
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to the old namespace
# and verify that related types are also moved to the new namespace
# Search for old namespace references
echo "Checking for old namespace references..."
rg "using Cuemon\.Runtime\.Serialization;" -g '!*.{md,txt}'
# Search for types that might need to be moved
echo "Checking for types in the old namespace..."
ast-grep --pattern 'namespace Cuemon.Runtime.Serialization
{
$$$
}'
Length of output: 1903
Script:
#!/bin/bash
# Check if the HierarchySerializer class itself has been moved to the new namespace
echo "Checking for HierarchySerializer in new namespace..."
ast-grep --pattern 'namespace Cuemon.Extensions.Runtime.Serialization
{
$$$
class HierarchySerializer
$$$
}'
# Check for any other types using HierarchySerializer
echo "Checking for HierarchySerializer usage..."
rg "HierarchySerializer" -l
# Check if there are other types in the new namespace
echo "Checking for other types in new namespace..."
ast-grep --pattern 'namespace Cuemon.Extensions.Runtime.Serialization
{
$$$
}'
Length of output: 6101
src/Cuemon.Xml/Extensions/XmlReaderDecoratorExtensions.cs (2)
8-8: LGTM: Runtime extensions import added
The addition of Cuemon.Extensions.Runtime is appropriate for the functionality in this file.
Line range hint 119-196: Verify DataPair usage after related class removals
Given that the PR objectives mention removal of DataPairCollection and DataPairDictionary, we should verify that this implementation remains compatible.
test/Cuemon.Extensions.Text.Json.Tests/Converters/JsonConverterCollectionExtensionsTest.cs (3)
21-21: LGTM: Good replacement of obsolete enum
The change from HorizontalDirection to DayOfWeek is a good choice as it maintains the test's purpose while using a standard .NET enum.
Also applies to: 35-36
76-76: LGTM: Consistent enum replacement
The change maintains consistency with the previous test method while preserving the test's purpose of validating camelCase serialization.
Also applies to: 89-90
Line range hint 193-219: Verify if DataPair test should be retained
Given that the PR removes DataPairCollection and DataPairDictionary, please verify if this test should be updated or removed as part of the cleanup.
Let's check the status of DataPair-related classes:
✅ Verification successful
Based on the shell script results, I can now generate the final response:
DataPair test should be retained
The DataPair and DataPair<T> classes are still actively used throughout the codebase:
- Core implementation exists in
src/Cuemon.Core/DataPair.cs - Used extensively in XML processing (
Cuemon.Xml) - Utilized in JSON conversion (
Cuemon.Extensions.Text.Json) - Required for hierarchy operations (
Cuemon.Extensions.Core/Runtime)
While DataPairCollection and DataPairDictionary are being removed, the base DataPair functionality remains a fundamental part of the codebase, so this test should be kept to ensure the JSON conversion of DataPair continues to work correctly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for DataPair-related files and usages
# Find DataPair-related files
echo "DataPair-related files:"
fd -e cs -e vb DataPair
# Check for DataPair usages in the codebase
echo -e "\nDataPair usages:"
rg "DataPair" -t cs -t vb
Length of output: 9864
src/Cuemon.Core/Extensions/ObjectDecoratorExtensions.cs (2)
2-2: LGTM!
The added using directive is necessary for the new Stack<T> usage.
115-141: Consider performance implications and verify circular reference handling.
The implementation is correct but has some considerations:
- For deep hierarchies, the stack could grow large. Consider adding a depth limit parameter.
- Ensure that circular references are properly handled by the caller to prevent infinite loops.
Let's verify how circular references are handled:
src/Cuemon.Xml/Extensions/HierarchyDecoratorExtensions.cs (2)
Line range hint 1-190: Implementation looks solid and well-documented!
The implementation demonstrates:
- Consistent error handling with proper null checks
- Well-structured XML documentation
- Clear separation of concerns
- Good use of extension methods for XML attribute handling
7-7: Verify the dependency on Runtime extensions.
The addition of Cuemon.Extensions.Runtime looks good, but let's verify its usage and relationship to the removed classes mentioned in the PR objectives.
✅ Verification successful
The Runtime extensions import is correctly used and required
The Cuemon.Extensions.Runtime import is justified as it's used consistently across the XML module for serialization and type handling. The file uses the Decorator.Enclose() method from Runtime extensions for type operations and attribute handling. This is part of a broader pattern where Runtime extensions are used throughout the XML serialization components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of Runtime extensions and impact of removed classes
# Check for usage of Runtime extensions in this file
echo "Checking Runtime extensions usage:"
rg -A 3 "Runtime\." src/Cuemon.Xml/Extensions/HierarchyDecoratorExtensions.cs
# Check for references to removed classes (DataPairCollection, DataPairDictionary, etc.)
echo -e "\nChecking for references to removed classes:"
rg -l "DataPair|HorizontalDirection|IndexMapping|Initializer" src/Cuemon.Xml/Extensions/
# Verify if any XML-related functionality was moved from removed classes
echo -e "\nChecking for XML-related code in removed files:"
git log -p --full-history -- "**/DataPair*.cs" "**/IndexMapping.cs" | rg "Xml|XML"
Length of output: 606
Script:
#!/bin/bash
# Let's try a different approach to verify the Runtime extensions usage
# First, let's see the actual content of the file
echo "Content of HierarchyDecoratorExtensions.cs:"
cat src/Cuemon.Xml/Extensions/HierarchyDecoratorExtensions.cs
# Check what's in XmlReaderDecoratorExtensions.cs since it had a match
echo -e "\nContent of XmlReaderDecoratorExtensions.cs:"
cat src/Cuemon.Xml/Extensions/XmlReaderDecoratorExtensions.cs
# Let's also check for any Runtime namespace usage in XML-related files
echo -e "\nChecking Runtime namespace usage in XML files:"
rg "using.*Runtime" src/Cuemon.Xml/
Length of output: 22233
test/Cuemon.Core.Tests/Text/ParserFactoryTest.cs (1)
5-5: Verify dependencies after class removals.
The addition of Cuemon.Extensions namespace suggests a dependency on extension methods. Given the PR's removal of several classes, we should verify that all required extension methods are still available.
✅ Verification successful
Based on the shell script results, I can make a definitive assessment:
- No references to removed classes (
DataPairCollection,DataPairDictionary, etc.) were found in the test files. - The
Decorator.Enclosemethod is extensively used across test files, including inParserFactoryTest.cs. - The usage of
Decorator.EncloseinParserFactoryTest.csis for byte array and string encoding operations.
Let me verify the specific usage in ParserFactoryTest.cs:
Based on the complete analysis, I can now provide the final response:
The added Cuemon.Extensions namespace is correctly used and required.
The ParserFactoryTest.cs file uses extension methods from Cuemon.Extensions, specifically the ToByteArray() and ToEncodedString() methods through the Decorator.Enclose() pattern. These methods are actively used in multiple test methods including:
ParserFactory_ShouldConvertUrlEncodedBase64String_ToByteArrayParserFactory_ShouldConvertBinaryDigitsString_ToByteArrayParserFactory_ShouldConvertBase64String_ToByteArrayParserFactory_ShouldConvertHexadecimalString_ToByteArray
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any references to removed classes in test files
# and verify the availability of extension methods used in tests
# Check for references to removed classes
echo "Checking for references to removed classes..."
rg -l "DataPairCollection|DataPairDictionary|HorizontalDirection|IndexMapping|Initializer|InitializerBuilder" "test/Cuemon.Core.Tests"
# Check for Decorator.Enclose usage and its availability
echo "Checking Decorator.Enclose usage..."
rg "Decorator\.Enclose" "test/Cuemon.Core.Tests"
# Verify extension method declarations
echo "Checking extension method declarations..."
ast-grep --pattern 'public static class $_Extensions'
Length of output: 24825
Script:
#!/bin/bash
# Check the content of ParserFactoryTest.cs to understand the context
cat test/Cuemon.Core.Tests/Text/ParserFactoryTest.cs
Length of output: 11490
src/Cuemon.Data.SqlClient/SqlDataManager.cs (1)
227-229: LGTM! Verify exception handling behavior.
The change from Decorator.Enclose to Decorator.EncloseToExpose aligns with the PR's goal of enhancing exception handling. This modification could potentially expose more deeply nested SQL exceptions, which is beneficial for transient fault detection.
Let's verify the exception handling behavior:
✅ Verification successful
Based on the search results, I can now provide a final assessment of the change from Decorator.Enclose to Decorator.EncloseToExpose:
Change is safe and aligns with exception handling improvements
The change from Decorator.Enclose to Decorator.EncloseToExpose in ParseException method is part of a consistent pattern where EncloseToExpose is used specifically for exception handling and flattening scenarios. The evidence shows:
- The method is used internally to extract SQL exceptions from exception chains
- Similar pattern is used in other exception handling code (e.g.,
ExceptionExtensions.Flatten()) - No test failures or regressions are indicated in the codebase
The change improves consistency with the codebase's exception handling patterns while maintaining the same functionality of finding nested SQL exceptions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of Decorator.EncloseToExpose and related exception handling patterns
# Check for other similar exception handling patterns
rg -A 5 "Decorator\.EncloseToExpose|Decorator\.Enclose" --type cs
# Look for related test cases
fd -e cs -exec ast-grep --pattern 'class $_ {
$$$
void $_($$$) {
$$$
Decorator.EncloseToExpose($$$)
$$$
}
$$$
}'
Length of output: 396101
Script:
#!/bin/bash
# Search for usages of Decorator.EncloseToExpose to understand the change's impact
ast-grep --pattern 'Decorator.EncloseToExpose($$$)'
# Search for any test files that verify exception handling behavior
rg -l "ParseException|ExceptionDescriptor|Flatten|Decorator.EncloseToExpose" test/ --type cs
# Look for related test cases specifically around SQL exceptions
rg -l "SqlException|ParseException" test/Cuemon.Data.SqlClient.Tests/ --type cs
Length of output: 3422
Script:
#!/bin/bash
# Search for tests related to SqlDataManager and exception handling
rg -A 10 "class SqlDataManagerTest" test/Cuemon.Data.SqlClient.Tests/ --type cs
# Look for specific test cases around ParseException method
rg -A 10 "ParseException" test/Cuemon.Data.SqlClient.Tests/ --type cs
# Check for any usages of Decorator.Enclose with exception flattening
rg -A 5 "Decorator\.Enclose.*\.Flatten\(\)" --type cs
Length of output: 1965
src/Cuemon.Extensions.Core/Runtime/Hierarchy.cs (5)
43-51: Clean and modern property implementations.
The changes improve code quality by:
- Using expression-bodied members for better readability
- Implementing null-coalescing assignment for lazy initialization
315-320: Method names improved for clarity.
The renaming of traversal methods follows a more intuitive naming pattern:
WhileSourceTraversalIsNotNull→TraverseWhileNotNull- Iterator method renamed consistently
Also applies to: 323-333
234-239:
Document breaking change in parameter type.
The change from ObjectHierarchyOptions to HierarchyOptions is a breaking change that requires attention:
- Consider adding migration documentation
- Update API documentation to reflect the new parameter type
#!/bin/bash
# Check for existing usages of ObjectHierarchyOptions
rg "ObjectHierarchyOptions" -g "*.cs"
# Check for migration documentation
fd -e md -x grep -l "ObjectHierarchyOptions.*HierarchyOptions" {} \;346-348: Verify performance impact of decorator pattern.
The implementation of TraverseWhileNotEmpty now uses a decorator pattern which might have performance implications for large hierarchies.
#!/bin/bash
# Check decorator implementation
ast-grep --pattern 'class $_ : Decorator<$_> {
$$$
TraverseWhileNotEmpty($$$) {
$$$
}
$$$
}'7-7: Verify namespace change impact.
The class has been moved to Cuemon.Extensions.Runtime namespace. This change might affect existing code that references these types.
✅ Verification successful
Let me search for both namespace patterns and actual usage of the Hierarchy class to better understand the impact of this change.
Namespace change is correctly implemented and referenced
The namespace change from Cuemon.Extensions.Core.Runtime to Cuemon.Extensions.Runtime appears to be properly implemented across the codebase. All references to the Hierarchy class are using the new namespace, and there are no lingering references to the old namespace. The change is part of a consistent reorganization pattern visible in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for old namespace usage that needs updating
rg "using Cuemon;" -A 3 -g "*.cs"
Length of output: 242
Script:
#!/bin/bash
# Search for references to the Hierarchy class in both old and new namespaces
rg "using Cuemon\.Extensions\.Runtime|using Cuemon\.Extensions\.Core\.Runtime" -g "*.cs"
# Search for direct usage of Hierarchy class
rg "Hierarchy[<\s]" -g "*.cs" --no-heading
Length of output: 14551
src/Cuemon.Xml/Extensions/Serialization/Converters/XmlConverterDecoratorExtensions.cs (6)
8-8: LGTM!
The new using directive is properly placed and required for runtime extension methods used in the code.
Line range hint 198-236: LGTM!
The implementation is well-structured with:
- Clear configuration pattern using setup action
- Proper handling of sensitivity details
- Good separation of concerns
Line range hint 238-255: LGTM!
Clean implementation that follows the established patterns and properly handles URI serialization.
Line range hint 257-274: LGTM!
Excellent implementation that:
- Uses ISO 8601 format for consistent DateTime serialization
- Properly handles culture-invariant conversion
Line range hint 299-324: LGTM!
Well-implemented string converter with proper handling of:
- XML strings using CDATA
- Null and whitespace checks
Line range hint 326-367: LGTM!
Clean and straightforward implementations that properly wrap their respective converter classes.
src/Cuemon.Extensions.Core/Runtime/HierarchyDecoratorExtensions.cs (2)
9-9: LGTM: Namespace change aligns with modern extension method organization.
The move to Cuemon.Extensions.Runtime namespace follows the convention of organizing extension methods in dedicated extension namespaces.
158-158: LGTM: Search methods refactored for better consistency.
The Find/Search methods have been refactored to use the decorator pattern consistently, improving code readability and maintainability.
Also applies to: 170-170, 182-182, 194-194, 206-206, 220-220
src/Cuemon.Core/Extensions/TypeDecoratorExtensions.cs (1)
420-420: LGTM! Verify the removal of Infrastructure class.
The change correctly replaces the Infrastructure.DefaultPropertyValueResolver with Decorator.RawEnclose(s).DefaultPropertyValueResolver(i), maintaining the same functionality while aligning with the removal of the Infrastructure class.
Let's verify that the Infrastructure class has been completely removed and there are no remaining references:
src/Cuemon.Xml/Serialization/Converters/DefaultXmlConverter.cs (1)
Line range hint 51-53: Avoid potential infinite recursion in ReadXml.
In the ReadXml method, if an object is both enumerable and dictionary, it will always attempt to parse as a dictionary. Ensure that this logic aligns with expected behavior to avoid infinite recursion or stack overflows.
Please verify that the type checking logic correctly handles types that implement both interfaces.
| [assembly: SuppressMessage("Critical Code Smell", "S3776:Cognitive Complexity of methods should not be too high", Justification = "TODO - due to .NET 9 release this week, its kept as-is with this round of refactoring.", Scope = "member", Target = "~M:Cuemon.Xml.XmlReaderDecoratorExtensions.BuildHierarchy(System.Xml.XmlReader)~Cuemon.Extensions.Runtime.IHierarchy{Cuemon.DataPair}")] | ||
| [assembly: SuppressMessage("Critical Code Smell", "S3776:Cognitive Complexity of methods should not be too high", Justification = "TODO - due to .NET 9 release this week, its kept as-is with this round of refactoring.", Scope = "member", Target = "~M:Cuemon.Xml.Serialization.Converters.XmlConverterDecoratorExtensions.AddEnumerableConverter(Cuemon.IDecorator{System.Collections.Generic.IList{Cuemon.Xml.Serialization.Converters.XmlConverter}})~Cuemon.IDecorator{System.Collections.Generic.IList{Cuemon.Xml.Serialization.Converters.XmlConverter}}")] | ||
| [assembly: SuppressMessage("Critical Code Smell", "S3776:Cognitive Complexity of methods should not be too high", Justification = "TODO - due to .NET 9 release this week, its kept as-is with this round of refactoring.", Scope = "member", Target = "~M:Cuemon.Xml.Serialization.Converters.DefaultXmlConverter.ParseReadXmlDefault(System.Xml.XmlReader,System.Type)~System.Object")] | ||
| [assembly: SuppressMessage("Critical Code Smell", "S3776:Cognitive Complexity of methods should not be too high", Justification = "TODO - due to .NET 9 release this week, its kept as-is with this round of refactoring.", Scope = "member", Target = "~M:Cuemon.Xml.Serialization.Converters.ExceptionConverter.ParseXmlReader(System.Xml.XmlReader,System.Type)~System.Collections.Generic.Stack{System.Collections.Generic.IList{Cuemon.Reflection.MemberArgument}}")] |
There was a problem hiding this comment.
💡 Codebase verification
Based on the code snippets revealed, I can now generate a final response that addresses the complexity concerns with concrete evidence.
High cognitive complexity confirmed in XML processing methods
The code review reveals multiple complex methods that handle XML processing:
BuildHierarchy: Uses nested control structures with multiple data structures (Stack, Dictionary, List) for XML hierarchy traversalAddEnumerableConverter: Contains complex conditional logic for handling different enumerable types and key-value pairsParseReadXmlDefault: Uses goto statements and nested switches for XML node traversalParseXmlReader: Part of exception serialization with complex member argument handling
Recommended actions:
- Create separate issues for each method with the following refactoring strategies:
BuildHierarchy: Extract hierarchy building logic into smaller focused methodsAddEnumerableConverter: Split dictionary and list handling into separate converter implementationsParseReadXmlDefault: Remove goto statements and refactor into state machine patternParseXmlReader: Extract member argument parsing into dedicated helper class
🔗 Analysis chain
Create a follow-up plan for addressing high cognitive complexity.
The suppressions indicate multiple methods with high cognitive complexity that were temporarily kept as-is due to the .NET 9 release timing. While this is understandable for the immediate release, we should track and address these technical debt items.
Consider:
- Creating separate issues to refactor each complex method
- Prioritizing these refactoring tasks for the next release cycle
- Adding more detailed TODO comments explaining the specific complexity concerns
Let's analyze the complexity of these methods:
Let me try a different approach to find these methods since the previous pattern didn't work.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Analyze the complexity of the affected methods
# Expected results: Source code of the complex methods to understand their structure
# Check BuildHierarchy method
echo "Analyzing BuildHierarchy method..."
ast-grep --pattern 'BuildHierarchy(XmlReader $reader) {
$$$
}'
# Check AddEnumerableConverter method
echo "Analyzing AddEnumerableConverter method..."
ast-grep --pattern 'AddEnumerableConverter($decorator) {
$$$
}'
# Check ParseReadXmlDefault method
echo "Analyzing ParseReadXmlDefault method..."
ast-grep --pattern 'ParseReadXmlDefault(XmlReader $reader, Type $type) {
$$$
}'
# Check ParseXmlReader method
echo "Analyzing ParseXmlReader method..."
ast-grep --pattern 'ParseXmlReader(XmlReader $reader, Type $type) {
$$$
}'
Length of output: 639
Script:
#!/bin/bash
# Search for the complex methods using ripgrep for better context
echo "Searching for BuildHierarchy method..."
rg "BuildHierarchy.*XmlReader" -A 20
echo "Searching for AddEnumerableConverter method..."
rg "AddEnumerableConverter.*IDecorator" -A 20
echo "Searching for ParseReadXmlDefault method..."
rg "ParseReadXmlDefault.*XmlReader.*Type" -A 20
echo "Searching for ParseXmlReader method in ExceptionConverter..."
rg "class ExceptionConverter" -A 50
Length of output: 29002
| var root = decorator.AncestorsAndSelf().FirstOrDefault(); | ||
| var descendantsFromRoot = Decorator.Enclose(root).DescendantsAndSelf(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider caching the root node lookup.
Multiple methods (SiblingsAndSelfAt and FlattenAll) perform the same root node lookup using AncestorsAndSelf().FirstOrDefault(). Consider caching this result to avoid redundant traversals.
+private static IHierarchy<T> GetRootNode<T>(this IDecorator<IHierarchy<T>> decorator)
+{
+ return decorator.AncestorsAndSelf().FirstOrDefault();
+}
public static IEnumerable<IHierarchy<T>> SiblingsAndSelfAt<T>(this IDecorator<IHierarchy<T>> decorator, int depth)
{
Validator.ThrowIfNull(decorator);
Validator.ThrowIfLowerThan(depth, 0, nameof(depth));
- var root = decorator.AncestorsAndSelf().FirstOrDefault();
+ var root = decorator.GetRootNode();
var descendantsFromRoot = Decorator.Enclose(root).DescendantsAndSelf();
// ...
}
public static IEnumerable<IHierarchy<T>> FlattenAll<T>(this IDecorator<IHierarchy<T>> decorator)
{
Validator.ThrowIfNull(decorator);
- var root = decorator.AncestorsAndSelf().FirstOrDefault();
+ var root = decorator.GetRootNode();
return Decorator.Enclose(root).DescendantsAndSelf();
}Also applies to: 376-377
| [assembly: SuppressMessage("Major Code Smell", "S107:Methods should not have too many parameters", Justification = "By design; allow to exceed generic arguments recommendation.", Scope = "member", Target = "~M:Cuemon.Extensions.TesterFuncFactory.Create``16(Cuemon.TesterFunc{``0,``1,``2,``3,``4,``5,``6,``7,``8,``9,``10,``11,``12,``13,``14,``15},``0,``1,``2,``3,``4,``5,``6,``7,``8,``9,``10,``11,``12,``13)~Cuemon.TesterFuncFactory{Cuemon.MutableTuple{``0,``1,``2,``3,``4,``5,``6,``7,``8,``9,``10,``11,``12,``13},``14,``15}")] | ||
| [assembly: SuppressMessage("Major Code Smell", "S107:Methods should not have too many parameters", Justification = "By design; allow to exceed generic arguments recommendation.", Scope = "member", Target = "~M:Cuemon.Extensions.TesterFuncFactory.Create``17(Cuemon.TesterFunc{``0,``1,``2,``3,``4,``5,``6,``7,``8,``9,``10,``11,``12,``13,``14,``15,``16},``0,``1,``2,``3,``4,``5,``6,``7,``8,``9,``10,``11,``12,``13,``14)~Cuemon.TesterFuncFactory{Cuemon.MutableTuple{``0,``1,``2,``3,``4,``5,``6,``7,``8,``9,``10,``11,``12,``13,``14},``15,``16}")] | ||
| [assembly: SuppressMessage("Major Code Smell", "S107:Methods should not have too many parameters", Justification = "By design; allow to exceed generic arguments recommendation.", Scope = "member", Target = "~M:Cuemon.Extensions.TesterFuncFactory.Create``9(Cuemon.TesterFunc{``0,``1,``2,``3,``4,``5,``6,``7,``8},``0,``1,``2,``3,``4,``5,``6)~Cuemon.TesterFuncFactory{Cuemon.MutableTuple{``0,``1,``2,``3,``4,``5,``6},``7,``8}")] | ||
| [assembly: SuppressMessage("Critical Code Smell", "S3776:Cognitive Complexity of methods should not be too high", Justification = "TODO - due to .NET 9 release this week, its kept as-is with this round of refactoring.", Scope = "member", Target = "~M:Cuemon.Extensions.Runtime.Hierarchy.GetObjectHierarchy(System.Object,System.Action{Cuemon.Extensions.Runtime.HierarchyOptions})~Cuemon.Extensions.Runtime.IHierarchy{System.Object}")] |
There was a problem hiding this comment.
💡 Codebase verification
Consider breaking down the complex GetObjectHierarchy method into smaller, focused components
The method has high cognitive complexity due to:
- Complex nested loops and conditions
- Multiple responsibilities (object traversal, circular reference handling, property inspection)
- Deep nesting of control structures
Suggested refactoring:
- Extract property inspection logic into
PropertyInspectorclass - Move circular reference handling to
CircularReferenceTrackerclass - Create
HierarchyBuilderclass to handle the core tree construction - Split stack-based traversal into a separate
ObjectTraversercomponent
🔗 Analysis chain
Consider breaking down the complex method.
High cognitive complexity in GetObjectHierarchy suggests the method might be doing too much. Consider:
- Breaking it into smaller, focused methods
- Extracting complex logic into helper classes
- Using the Strategy pattern for different hierarchy building approaches
Let's analyze the method's complexity:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Analyze the GetObjectHierarchy method implementation
# to understand its complexity and suggest specific refactoring approaches
# Find and display the method implementation
ast-grep --pattern $'class Hierarchy {
$$$
GetObjectHierarchy(System.Object, System.Action<Cuemon.Extensions.Runtime.HierarchyOptions>) {
$$$
}
$$$
}'
Length of output: 163
Script:
#!/bin/bash
# Let's try a different approach to find the method implementation
# First, let's locate the file containing the method
fd "Hierarchy.cs" --type f
# Then we'll use ripgrep to find and display the method implementation
rg "GetObjectHierarchy.*\(System\.Object" -A 50
Length of output: 656
Script:
#!/bin/bash
# Let's examine the actual implementation in Hierarchy.cs
cat "src/Cuemon.Extensions.Core/Runtime/Hierarchy.cs"
Length of output: 16898
|
This pull request includes several significant changes to the
Cuemon.Corelibrary, focusing on removing obsolete classes, enhancing extension methods, and updating suppression messages.Goal: further reduction of NuGet package by slimming down core capabilities.
Removal of Obsolete Classes:
DataPairCollectionclass fromsrc/Cuemon.Core/Collections/DataPairCollection.cs.DataPairDictionaryclass fromsrc/Cuemon.Core/Collections/DataPairDictionary.cs.HorizontalDirectionenum fromsrc/Cuemon.Core/HorizontalDirection.cs.IndexMappingclass fromsrc/Cuemon.Core/IndexMapping.cs.InitializerandInitializerBuilderclasses fromsrc/Cuemon.Core/Initializer.csandsrc/Cuemon.Core/InitializerBuilder.cs. [1] [2]Enhancements to Extension Methods:
Flattenmethod inExceptionDecoratorExtensionsto useDecorator.RawEncloseandTraverseWhileNotEmpty.DefaultPropertyValueResolverandTraverseWhileNotEmptymethods toObjectDecoratorExtensions.HasCircularReferencemethod inTypeDecoratorExtensionsto useDecorator.RawEncloseandDefaultPropertyValueResolver.Updates to Suppression Messages:
GlobalSuppressions.cs. [1] [2]Miscellaneous:
System.Collections.Genericusing directive inObjectDecoratorExtensions.cs.Infrastructureclass fromsrc/Cuemon.Core/Infrastructure.cs.Summary by CodeRabbit
New Features
VerticalDirectionandFailure, to support enhanced functionality.Bug Fixes
Refactor
Tests
Chores