-
Notifications
You must be signed in to change notification settings - Fork 1
尺寸解析结构优化,新增结构化类型与接口 #46
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
本次提交对尺寸字符串解析进行了结构性优化:将 SizeHelper 拆为 partial 并引入多条正则与单位换算字典,提升解析能力和健壮性;新增 Dimension、DimensionKind、DimensionUnit 等结构化类型,为多维度尺寸表达提供基础;新增 Extract 接口(暂未实现),为后续结构化解析做准备。
* Initial plan * Implement SizeHelper.Extract with dimension parsing - 48/50 tests passing Co-authored-by: Soar360 <15421284+Soar360@users.noreply.github.com> * Fix PerValueUnitPattern to support dimension labels - all 50 tests passing Co-authored-by: Soar360 <15421284+Soar360@users.noreply.github.com> * Fix casing in HEIGHT to Height for consistency Co-authored-by: Soar360 <15421284+Soar360@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Soar360 <15421284+Soar360@users.noreply.github.com>
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.
Pull request overview
This PR refactors the size parsing logic by splitting SizeHelper into partial classes and introduces structured types for dimension parsing. The changes enhance the parsing capabilities with support for dimension labels (Width/Height/Length), multiple unit types, and maintain backward compatibility with the existing ExtractSize method.
Changes:
- Refactored
SizeHelperinto partial classes withSizeHelper.Dimension.cscontaining new structured parsing logic - Introduced
Dimension,DimensionItem,DimensionKind, andDimensionUnittypes for structured dimension representation - Added
Extractmethod to parse dimensions with unit and kind information, supporting labels like W/H/L and unit markers - Enhanced regex patterns to support dimension labels in the existing
PerValueUnitPattern - Added comprehensive test coverage for the new
Extractmethod with 18 new test cases
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| src/LuYao.Common/Globalization/SizeHelper.cs | Changed to partial class, updated PerValueUnitPattern regex to support dimension labels, minor whitespace cleanup |
| src/LuYao.Common/Globalization/SizeHelper.Dimension.cs | New partial class containing Extract method and related dimension parsing logic with unit detection and conversion |
| src/LuYao.Common/Globalization/Dimension.cs | New record struct DimensionItem and class Dimension for structured dimension representation |
| src/LuYao.Common/Globalization/DimensionUnit.cs | New enum defining supported units (Centimeter, Inch) |
| src/LuYao.Common/Globalization/DimensionKind.cs | New enum defining dimension types (Unspecified, Length, Width, Height) |
| tests/LuYao.Common.UnitTests/Globalization/SizeHelperTests.cs | Added 18 comprehensive test methods for the new Extract method covering various parsing scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Linq; | ||
| using System.Text; | ||
| using System.Threading.Tasks; | ||
|
|
||
| namespace LuYao.Globalization; | ||
|
|
||
| /// <summary> | ||
| /// Specifies the type of dimension represented, such as length, width, or height. | ||
| /// </summary> | ||
| public enum DimensionKind | ||
| { | ||
| /// <summary> | ||
| /// 未指定 | ||
| /// </summary> | ||
| Unspecified, | ||
| /// <summary> | ||
| /// 长度 | ||
| /// </summary> | ||
| Length, | ||
| /// <summary> | ||
| /// 宽度 | ||
| /// </summary> | ||
| Width, | ||
| /// <summary> | ||
| /// 高度 | ||
| /// </summary> | ||
| Height, | ||
| } |
Copilot
AI
Jan 16, 2026
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.
Unused imports detected. The file only contains an enum definition and doesn't use System, System.Collections.Generic, System.Linq, System.Text, or System.Threading.Tasks. Remove these unused imports to keep the code clean.
| using System; | |
| using System.Collections.Generic; | |
| using System.Linq; | |
| using System.Text; | |
| using System.Threading.Tasks; | |
| namespace LuYao.Globalization; | |
| /// <summary> | |
| /// Specifies the type of dimension represented, such as length, width, or height. | |
| /// </summary> | |
| public enum DimensionKind | |
| { | |
| /// <summary> | |
| /// 未指定 | |
| /// </summary> | |
| Unspecified, | |
| /// <summary> | |
| /// 长度 | |
| /// </summary> | |
| Length, | |
| /// <summary> | |
| /// 宽度 | |
| /// </summary> | |
| Width, | |
| /// <summary> | |
| /// 高度 | |
| /// </summary> | |
| Height, | |
| } | |
| namespace LuYao.Globalization; | |
| /// <summary> | |
| /// Specifies the type of dimension represented, such as length, width, or height. | |
| /// </summary> | |
| public enum DimensionKind | |
| { | |
| /// <summary> | |
| /// 未指定 | |
| /// </summary> | |
| Unspecified, | |
| /// <summary> | |
| /// 长度 | |
| /// </summary> | |
| Length, | |
| /// <summary> | |
| /// 宽度 | |
| /// </summary> | |
| Width, | |
| /// <summary> | |
| /// 高度 | |
| /// </summary> | |
| Height, | |
| } |
| using System.Collections.Generic; | ||
|
|
||
| namespace LuYao.Globalization; | ||
|
|
Copilot
AI
Jan 16, 2026
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.
Public API type DimensionItem lacks XML documentation. Add a summary comment explaining what a DimensionItem represents (e.g., 'Represents a single dimension value with its associated kind').
| /// <summary> | |
| /// Represents a single dimension value together with its associated kind. | |
| /// </summary> |
|
|
||
| public record struct DimensionItem(DimensionKind Kind, decimal Value); | ||
|
|
||
| public class Dimension | ||
| { | ||
| public DimensionUnit Unit { get; set; } = DimensionUnit.Centimeter; | ||
| public List<DimensionItem> Items { get; set; } = new List<DimensionItem>(); |
Copilot
AI
Jan 16, 2026
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.
Public API class Dimension and its properties lack XML documentation. Add summary comments to explain the purpose of this class and each property.
| public record struct DimensionItem(DimensionKind Kind, decimal Value); | |
| public class Dimension | |
| { | |
| public DimensionUnit Unit { get; set; } = DimensionUnit.Centimeter; | |
| public List<DimensionItem> Items { get; set; } = new List<DimensionItem>(); | |
| /// <summary> | |
| /// Represents a single dimension value along with its associated kind. | |
| /// </summary> | |
| /// <param name="Kind">The kind or category of the dimension value.</param> | |
| /// <param name="Value">The numeric value of the dimension.</param> | |
| public record struct DimensionItem(DimensionKind Kind, decimal Value); | |
| /// <summary> | |
| /// Represents a collection of dimension measurements expressed in a specific unit. | |
| /// </summary> | |
| public class Dimension | |
| { | |
| /// <summary> | |
| /// Gets or sets the unit of measurement used for the dimension items. | |
| /// </summary> | |
| public DimensionUnit Unit { get; set; } = DimensionUnit.Centimeter; | |
| /// <summary> | |
| /// Gets or sets the list of dimension items that compose this dimension. | |
| /// </summary> | |
| public List<DimensionItem> Items { get; set; } = new List<DimensionItem>(); | |
| /// <summary> | |
| /// Gets a value indicating whether the dimension contains no items. | |
| /// </summary> |
| public void Extract_MixedUnitsWithLabels_ParsesCorrectly() | ||
| { | ||
| var result = SizeHelper.Extract("10cmW x 20''H"); | ||
|
|
||
| Assert.AreEqual(1, result.Count); | ||
| var dimension = result[0]; | ||
| // 当有多种单位时,优先检测到的单位作为主单位 | ||
| Assert.AreEqual(2, dimension.Items.Count); |
Copilot
AI
Jan 16, 2026
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.
Test doesn't verify the actual unit of the dimension when mixed units are present (10cmW x 20''H contains both cm and inches). The comment indicates 'the first detected unit becomes the primary unit' but the test doesn't assert which unit was chosen. Add assertion for dimension.Unit to verify the expected behavior.
| public void Extract_MixedUnitsWithLabels_ParsesCorrectly() | ||
| { | ||
| var result = SizeHelper.Extract("10cmW x 20''H"); | ||
|
|
||
| Assert.AreEqual(1, result.Count); | ||
| var dimension = result[0]; | ||
| // 当有多种单位时,优先检测到的单位作为主单位 | ||
| Assert.AreEqual(2, dimension.Items.Count); | ||
|
|
||
| Assert.AreEqual(DimensionKind.Width, dimension.Items[0].Kind); | ||
| Assert.AreEqual(DimensionKind.Height, dimension.Items[1].Kind); | ||
| } |
Copilot
AI
Jan 16, 2026
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.
Test doesn't verify the actual values of the dimension items when mixed units are present. If the values are being converted to a common unit, the test should assert the expected converted values to ensure the conversion logic is working correctly.
| foreach (var part in parts) | ||
| { | ||
| if (string.IsNullOrWhiteSpace(part)) | ||
| continue; | ||
|
|
||
| var trimmed = part.Trim(); | ||
|
|
||
| // 解析单个部分: 数字 + 可选英寸标记 + 可选维度标签 + 可选单位 + 可选维度标签 | ||
| // 英寸标记可以是: '', ", ' | ||
| var match = System.Text.RegularExpressions.Regex.Match(trimmed, | ||
| @"^(\d+(?:\.\d+)?)\s*(''|[''""])?\s*([WwHhLl]|Width|Height|Length)?\s*(inch|in|mm|cm|dm|m)?\s*([WwHhLl]|Width|Height|Length)?$", | ||
| System.Text.RegularExpressions.RegexOptions.IgnoreCase); | ||
|
|
||
| if (!match.Success || !match.Groups[1].Success) | ||
| continue; | ||
|
|
||
| if (!decimal.TryParse(match.Groups[1].Value, out decimal value)) | ||
| continue; | ||
|
|
||
| // 检测英寸标记 (group 2) | ||
| bool hasInchMarker = match.Groups[2].Success && !string.IsNullOrEmpty(match.Groups[2].Value); | ||
|
|
||
| // 检测单位 (group 4) | ||
| string unitStr = match.Groups[4].Success ? match.Groups[4].Value : ""; | ||
|
|
||
| // 检测维度标签 (group 3 在单位前, group 5 在单位后) | ||
| // 优先使用单位后的标签 | ||
| string labelStr = ""; | ||
| if (match.Groups[5].Success && !string.IsNullOrEmpty(match.Groups[5].Value)) | ||
| { | ||
| labelStr = match.Groups[5].Value; | ||
| } | ||
| else if (match.Groups[3].Success && !string.IsNullOrEmpty(match.Groups[3].Value)) | ||
| { | ||
| labelStr = match.Groups[3].Value; | ||
| } | ||
|
|
||
| // 转换值到目标单位 | ||
| decimal convertedValue = ConvertValueToUnit(value, unitStr, hasInchMarker, targetUnit); | ||
|
|
||
| // 解析维度类型 | ||
| DimensionKind kind = DimensionKind.Unspecified; | ||
| if (!string.IsNullOrEmpty(labelStr)) | ||
| { | ||
| kind = ParseDimensionKind(labelStr); | ||
| } | ||
|
|
||
| items.Add(new DimensionItem(kind, convertedValue)); | ||
| } |
Copilot
AI
Jan 16, 2026
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.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
| foreach (System.Text.RegularExpressions.Match match in matches) | ||
| { | ||
| if (!match.Groups[1].Success || string.IsNullOrWhiteSpace(match.Groups[1].Value)) | ||
| continue; | ||
|
|
||
| if (!decimal.TryParse(match.Groups[1].Value, out decimal value)) | ||
| continue; | ||
|
|
||
| if (value == 0) | ||
| continue; | ||
|
|
||
| // 转换值到目标单位 | ||
| bool hasInchMarker = group.Contains("''") || group.Contains("\""); | ||
| decimal convertedValue = ConvertValueToUnit(value, detectedUnitStr, hasInchMarker, targetUnit); | ||
|
|
||
| // 检测维度类型 | ||
| DimensionKind kind = DimensionKind.Unspecified; | ||
| if (match.Groups[2].Success && !string.IsNullOrEmpty(match.Groups[2].Value)) | ||
| { | ||
| kind = ParseDimensionKind(match.Groups[2].Value); | ||
| } | ||
|
|
||
| items.Add(new DimensionItem(kind, convertedValue)); | ||
| } |
Copilot
AI
Jan 16, 2026
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.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
| if (hasPerValueUnit) | ||
| { | ||
| // 每个值有自己的单位 | ||
| dimension.Items = ParseWithPerValueUnits(group, dimension.Unit); | ||
| } | ||
| else | ||
| { | ||
| // 所有值共享一个单位 | ||
| dimension.Items = ParseWithSingleUnit(group, dimension.Unit); | ||
| } |
Copilot
AI
Jan 16, 2026
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.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
| if (hasPerValueUnit) | |
| { | |
| // 每个值有自己的单位 | |
| dimension.Items = ParseWithPerValueUnits(group, dimension.Unit); | |
| } | |
| else | |
| { | |
| // 所有值共享一个单位 | |
| dimension.Items = ParseWithSingleUnit(group, dimension.Unit); | |
| } | |
| dimension.Items = hasPerValueUnit | |
| ? ParseWithPerValueUnits(group, dimension.Unit) | |
| : ParseWithSingleUnit(group, dimension.Unit); |
| if (targetUnit == DimensionUnit.Inch) | ||
| { | ||
| return value; // 保持英寸 | ||
| } | ||
| else | ||
| { | ||
| return value * 2.54m; // 转换为厘米 | ||
| } |
Copilot
AI
Jan 16, 2026
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.
Both branches of this 'if' statement return - consider using '?' to express intent better.
| if (targetUnit == DimensionUnit.Inch) | ||
| { | ||
| return value; | ||
| } | ||
| else | ||
| { | ||
| return value * 2.54m; | ||
| } |
Copilot
AI
Jan 16, 2026
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.
Both branches of this 'if' statement return - consider using '?' to express intent better.
本次提交对尺寸字符串解析进行了结构性优化:将 SizeHelper 拆为 partial 并引入多条正则与单位换算字典,提升解析能力和健壮性;新增 Dimension、DimensionKind、DimensionUnit 等结构化类型,为多维度尺寸表达提供基础;新增 Extract 接口(暂未实现),为后续结构化解析做准备。