-
Notifications
You must be signed in to change notification settings - Fork 180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add additional XmlQualifiedName for guid #403
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #403 +/- ##
=======================================
Coverage 94.00% 94.01%
=======================================
Files 21 21
Lines 3021 3025 +4
Branches 479 479
=======================================
+ Hits 2840 2844 +4
Misses 119 119
Partials 62 62
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's a review and some improvements for your code:
Review of Changes:
- Variable Name Change: Changed
GuidQualifiedName
toGuidQualifiedNames
as it now holds an array ofXmlQualifiedName
. - Array Initialization: Initialized
GuidQualifiedNames
with two instances ofXmlQualifiedName
. - SchemaType Check Improvement: Changed
schemaType.IsDerivedFrom(GuidQualifiedName)
toGuidQualifiedNames.Any(schemaType.IsDerivedFrom)
for checking ifschemaType
is derived from any of theGuidQualifiedNames
.
Improvements:
-
Consistency in Naming: Ensure that variable names (
GuidQualifiedNames
) accurately reflect their purpose and are consistently used throughout the codebase. -
Clarity and Readability: Consider using more descriptive variable names where appropriate to improve code readability.
-
Error Handling: Implement error handling or logging mechanisms to manage scenarios where
schemaType
does not match any of the expectedGuidQualifiedNames
. -
Unit Testing: Write unit tests to validate the behavior of the
GetEffectiveType
method under various scenarios, including differentschemaType
inputs.
Consolidated Code (with improvements):
Type FromFallback() => configuration.UseIntegerDataTypeAsFallback && configuration.IntegerDataType != null ? configuration.IntegerDataType : typeof(string);
private static readonly XmlQualifiedName[] GuidQualifiedNames =
{
new XmlQualifiedName("guid", "http://microsoft.com/wsdl/types/"),
new XmlQualifiedName("guid", "http://schemas.microsoft.com/2003/10/Serialization/")
};
public static Type GetEffectiveType(this XmlSchemaDatatype type, GeneratorConfiguration configuration, IEnumerable<RestrictionModel> restrictions, XmlSchemaType schemaType, bool attribute = false)
{
Type resultType = attribute ? typeof(string) : type.ValueType;
if (GuidQualifiedNames.Any(schemaType.IsDerivedFrom))
{
resultType = typeof(Guid);
}
return resultType;
}
Summary:
- Naming and Readability: Improved by using descriptive names (
GuidQualifiedNames
) and ensuring consistency. - Functionality: Enhanced by using array initialization for
GuidQualifiedNames
and checking against any of these names inGetEffectiveType
. - Maintainability: Improved through clearer logic and potential error handling suggestions.
Feel free to integrate these suggestions into your codebase/
@mganss Just to make your life easier I created a PR