-
Notifications
You must be signed in to change notification settings - Fork 10
VS-99: Optimize MQL Generator to avoid having 1 using statement for each Bson Type #38
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
Conversation
| #pragma warning disable CS0169 // The field is never used | ||
| #pragma warning disable IDE0051 | ||
| private static readonly BsonTypeCustom123 s_dummyRef1; | ||
| private static readonly BsonTimeSpanCustom123 s_dummyRef2; | ||
| private static readonly BsonType s_dummyRef1; |
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.
Update the comment please to "// These fields are never used, they are needed to ensure that the relevant usings are not accidently removed"
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.
Done
|
|
||
| var fullTypeName = GetFullName(typeSymbol); | ||
| if (s_knownBsonTypes.TryGetValue(fullTypeName, out var knowTypeName)) | ||
| if (typeSymbol.IsSupportedBsonType() || typeSymbol.IsSupportedBsonSerializationOption()) |
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.
These methods perform ToDisplayString twice, while we already have fullTypeName.
Lets minimize the lookups and imbed IsSupportedBsonSerializationOption into IsSupportedBsonType (startsWith(NamespaceMongoDBBson)), and receive optional parameter fullTypeName.
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.
Done
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.
LGTM (notice the small comment)
| private static readonly HashSet<string> s_supportedBsonTypes = new() | ||
| { | ||
| "MongoDB.Bson.BsonDocument", | ||
| "MongoDB.Bson.BsonValue", |
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.
Alphabetical order
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.
Done
VS-99: Optimize MQL Generator to avoid having 1 using statement for each Bson Type