avoid casts by using pattern matching#4014
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors several type checks to use C# pattern matching (is T t) in order to avoid repeated casts and improve local readability in core TDS write/serialization paths.
Changes:
- Replace
is+ explicit cast patterns withis <Type> <local>pattern matching inTdsParser. - Use pattern matching in
SqlParametertype conversion logic to avoidascast. - Use pattern matching in
SqlBulkCopyconversion to avoid anXmlReadercast when wrapping inXmlDataFeed.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs | Replaces several is/cast sequences with pattern matching locals in RPC parameter writing, bulk copy size calculation, and value serialization. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs | Uses pattern matching to capture SqlParameter for InstanceDescriptor conversion. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs | Uses pattern matching for XmlReader handling when converting XML/JSON values to XmlDataFeed. |
| if (value is SqlChars sqlCharsA) | ||
| { | ||
| string sch = new string(((SqlChars)value).Value); | ||
| string sch = new string(sqlCharsA.Value); | ||
|
|
There was a problem hiding this comment.
The suffix "A" on sqlCharsA isn’t meaningful here and reduces readability. Since the pattern variable is scoped to the if block, consider renaming to sqlChars (or another descriptive name) without the letter suffix.
| if (value is SqlChars sqlCharsB) | ||
| { | ||
| return WriteCharArray(((SqlChars)value).Value, actualLength, offset, stateObj, canAccumulate: false); | ||
| return WriteCharArray(sqlCharsB.Value, actualLength, offset, stateObj, canAccumulate: false); |
There was a problem hiding this comment.
sqlCharsB uses a letter suffix that doesn’t convey meaning. Consider renaming to sqlChars to match usage elsewhere in the codebase and keep the pattern variable names consistent.
| if (value is SqlBinary sqlBinary2) | ||
| { | ||
| Buffer.BlockCopy(((SqlBinary)value).Value, offset, b, 0, actualLength); | ||
| Buffer.BlockCopy(sqlBinary2.Value, offset, b, 0, actualLength); |
There was a problem hiding this comment.
sqlBinary2 uses a numeric suffix that doesn’t add meaning (it’s already scoped to this block). Consider renaming to sqlBinary for clarity and consistency with other pattern variables in this file.
| if (value is SqlChars sqlCharsC) | ||
| { | ||
| String sch = new String(((SqlChars)value).Value); | ||
| String sch = new String(sqlCharsC.Value); | ||
| return SerializeEncodingChar(sch, actualLength, offset, _defaultEncoding); |
There was a problem hiding this comment.
sqlCharsC introduces a letter suffix that isn’t meaningful. Consider renaming to sqlChars (the variable is block-scoped) to improve readability.
| if (value is SqlChars sqlCharsD) | ||
| { | ||
| return SerializeCharArray(((SqlChars)value).Value, actualLength, offset); | ||
| return SerializeCharArray(sqlCharsD.Value, actualLength, offset); |
There was a problem hiding this comment.
sqlCharsD uses a letter suffix that doesn’t convey intent. Consider renaming to sqlChars to keep pattern variable naming consistent and easier to scan.
No description provided.