feat(Converter): add HexConverter/BinConverter static class#544
Merged
feat(Converter): add HexConverter/BinConverter static class#544
Conversation
Reviewer's GuideThis PR introduces two static converter classes (HexConverter and BinConverter) for bidirectional transformations between byte arrays and their hexadecimal or binary string representations, adds corresponding unit tests, and updates the project file to include the new DataConverter folder. Class diagram for new HexConverter and BinConverter static classesclassDiagram
class HexConverter {
+static string ToString(byte[]? bytes, string? separator = "-")
+static byte[] ToBytes(string str, string? separator = null, StringSplitOptions options = StringSplitOptions.None)
}
class BinConverter {
+static string ToString(byte[]? bytes, string? separator = "-")
+static byte[] ToBytes(string str, string? separator = null, StringSplitOptions options = StringSplitOptions.None)
}
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds two new static converter classes - HexConverter and BinConverter - to support conversion between byte arrays and their hexadecimal/binary string representations. These utilities address issue #543 by providing reusable conversion functionality for socket data processing.
- Implements HexConverter for byte array ↔ hexadecimal string conversions
- Implements BinConverter for byte array ↔ binary string conversions
- Includes comprehensive unit tests for both converters
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/extensions/BootstrapBlazor.Socket/DataConverter/HexConverter.cs | Implements hexadecimal conversion utilities with customizable separators |
| src/extensions/BootstrapBlazor.Socket/DataConverter/BinConverter.cs | Implements binary conversion utilities with customizable separators |
| test/UnitTestTcpSocket/HexConverterTest.cs | Unit tests covering HexConverter functionality including edge cases |
| test/UnitTestTcpSocket/BinConverterTest.cs | Unit tests covering BinConverter functionality including edge cases |
| src/extensions/BootstrapBlazor.Socket/BootstrapBlazor.Socket.csproj | Version bump from 9.0.1 to 9.0.2 |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Comment on lines
+16
to
+20
| /// <para>Converts a byte array to its hexadecimal string representation.</para> | ||
| /// </summary> | ||
| /// <param name="bytes">The byte array to convert.</param> | ||
| /// <param name="separator"></param> | ||
| /// <returns>A string containing the hexadecimal representation of the byte array.</returns> |
There was a problem hiding this comment.
The documentation comment incorrectly states 'hexadecimal string representation' when it should be 'binary string representation' for the BinConverter class.
Suggested change
| /// <para>Converts a byte array to its hexadecimal string representation.</para> | |
| /// </summary> | |
| /// <param name="bytes">The byte array to convert.</param> | |
| /// <param name="separator"></param> | |
| /// <returns>A string containing the hexadecimal representation of the byte array.</returns> | |
| /// <para>Converts a byte array to its binary string representation.</para> | |
| /// </summary> | |
| /// <param name="bytes">The byte array to convert.</param> | |
| /// <param name="separator"></param> | |
| /// <returns>A string containing the binary representation of the byte array.</returns> |
| /// </summary> | ||
| /// <param name="bytes">The byte array to convert.</param> | ||
| /// <param name="separator"></param> | ||
| /// <returns>A string containing the hexadecimal representation of the byte array.</returns> |
There was a problem hiding this comment.
The return documentation incorrectly states 'hexadecimal representation' when it should be 'binary representation' for the BinConverter class.
Suggested change
| /// <returns>A string containing the hexadecimal representation of the byte array.</returns> | |
| /// <returns>A string containing the binary representation of the byte array.</returns> |
| str = string.Join("", str.Split(separator, options).Select(i => i.PadLeft(8, '0'))); | ||
| } | ||
|
|
||
| // 把 Hex 形式的 str 转化为 byte[] |
There was a problem hiding this comment.
The comment incorrectly refers to 'Hex 形式的 str' (Hex format string) when it should refer to binary format string in the BinConverter class.
Suggested change
| // 把 Hex 形式的 str 转化为 byte[] | |
| // 把二进制形式的 str 转化为 byte[] |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Update BinConverter’s XML docs and comments to refer to binary instead of hexadecimal for both ToString and ToBytes methods.
- Simplify HexConverter.ToString by using string.Join on formatted bytes to handle custom or empty separators cleanly and avoid manual trimming logic.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Update BinConverter’s XML docs and comments to refer to binary instead of hexadecimal for both ToString and ToBytes methods.
- Simplify HexConverter.ToString by using string.Join on formatted bytes to handle custom or empty separators cleanly and avoid manual trimming logic.
## Individual Comments
### Comment 1
<location> `src/extensions/BootstrapBlazor.Socket/DataConverter/HexConverter.cs:39` </location>
<code_context>
+ sb.Append(b.ToString("X2"));
+ sb.Append(separator);
+ }
+ return sb.ToString(0, sb.Length - 1);
+ }
+
</code_context>
<issue_to_address>
Potential issue if separator is an empty string.
When separator is empty, the current logic omits the last byte. Please add a condition to handle empty separators to ensure correct output.
</issue_to_address>
### Comment 2
<location> `src/extensions/BootstrapBlazor.Socket/DataConverter/BinConverter.cs:43` </location>
<code_context>
+ // 把 str 内的 separator 符号替换掉
+ if (!string.IsNullOrEmpty(separator))
+ {
+ str = string.Join("", str.Split(separator, options).Select(i => i.PadLeft(8, '0')));
+ }
+
</code_context>
<issue_to_address>
Padding each split segment to 8 may mask input errors.
Validate segment lengths before padding to prevent masking malformed input.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
// 把 str 内的 separator 符号替换掉
if (!string.IsNullOrEmpty(separator))
{
str = string.Join("", str.Split(separator, options).Select(i => i.PadLeft(8, '0')));
}
=======
// 把 str 内的 separator 符号替换掉
if (!string.IsNullOrEmpty(separator))
{
var segments = str.Split(separator, options);
const int expectedLength = 8;
foreach (var segment in segments)
{
if (segment.Length > expectedLength)
{
throw new FormatException($"Segment '{segment}' length {segment.Length} exceeds expected length {expectedLength}.");
}
}
str = string.Join("", segments.Select(i => i.PadLeft(expectedLength, '0')));
}
>>>>>>> REPLACE
</suggested_fix>
### Comment 3
<location> `src/extensions/BootstrapBlazor.Socket/DataConverter/BinConverter.cs:47` </location>
<code_context>
+ }
+
+ // 把 Hex 形式的 str 转化为 byte[]
+ if (str.Length % 8 != 0)
+ {
+ throw new ArgumentException("The raw string cannot have an odd number of digits. 参数 str 位数不正确无法转化为 二进制字节数组", nameof(str));
</code_context>
<issue_to_address>
Exception message refers to 'odd number of digits' for binary conversion.
Update the exception message to specify that the string length must be a multiple of 8 for binary conversion.
</issue_to_address>
### Comment 4
<location> `test/UnitTestTcpSocket/BinConverterTest.cs:11` </location>
<code_context>
+
+public class BinConverterTest
+{
+ [Fact]
+ public void ToHexString_Null()
+ {
+ var actual = BinConverter.ToString(null);
+ Assert.Equal(string.Empty, actual);
+
+ actual = BinConverter.ToString([]);
+ Assert.Equal(string.Empty, actual);
+ }
+
+ [Fact]
+ public void ToBinString_Ok()
+ {
+ var data = new byte[] { 0x1A, 0x02 };
+ var actual = BinConverter.ToString(data);
+ Assert.Equal("00011010-00000010", actual);
+
+ actual = BinConverter.ToString(data, " ");
+ Assert.Equal("00011010 00000010", actual);
+ }
+
+ [Fact]
+ public void ToHexString_Exception()
+ {
+ var data = "00011010-00000010";
</code_context>
<issue_to_address>
Exception test does not cover valid binary string with odd length.
Please add a test for a raw binary string with odd length, such as "0001101", to verify the exception is correctly thrown in this case.
</issue_to_address>
### Comment 5
<location> `test/UnitTestTcpSocket/HexConverterTest.cs:11` </location>
<code_context>
+
+public class BinConverterTest
+{
+ [Fact]
+ public void ToHexString_Null()
+ {
+ var actual = BinConverter.ToString(null);
+ Assert.Equal(string.Empty, actual);
+
+ actual = BinConverter.ToString([]);
+ Assert.Equal(string.Empty, actual);
+ }
+
+ [Fact]
+ public void ToBinString_Ok()
+ {
+ var data = new byte[] { 0x1A, 0x02 };
+ var actual = BinConverter.ToString(data);
+ Assert.Equal("00011010-00000010", actual);
+
+ actual = BinConverter.ToString(data, " ");
+ Assert.Equal("00011010 00000010", actual);
+ }
+
+ [Fact]
+ public void ToHexString_Exception()
+ {
+ var data = "00011010-00000010";
</code_context>
<issue_to_address>
Exception test does not cover valid hex string with odd length and with separators.
Please add a test with a hex string containing separators that results in an odd digit count after removing separators, to verify the exception is correctly thrown in this case.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| sb.Append(b.ToString("X2")); | ||
| sb.Append(separator); | ||
| } | ||
| return sb.ToString(0, sb.Length - 1); |
There was a problem hiding this comment.
issue: Potential issue if separator is an empty string.
When separator is empty, the current logic omits the last byte. Please add a condition to handle empty separators to ensure correct output.
Comment on lines
+40
to
+44
| // 把 str 内的 separator 符号替换掉 | ||
| if (!string.IsNullOrEmpty(separator)) | ||
| { | ||
| str = string.Join("", str.Split(separator, options).Select(i => i.PadLeft(8, '0'))); | ||
| } |
There was a problem hiding this comment.
suggestion (bug_risk): Padding each split segment to 8 may mask input errors.
Validate segment lengths before padding to prevent masking malformed input.
Suggested change
| // 把 str 内的 separator 符号替换掉 | |
| if (!string.IsNullOrEmpty(separator)) | |
| { | |
| str = string.Join("", str.Split(separator, options).Select(i => i.PadLeft(8, '0'))); | |
| } | |
| // 把 str 内的 separator 符号替换掉 | |
| if (!string.IsNullOrEmpty(separator)) | |
| { | |
| var segments = str.Split(separator, options); | |
| const int expectedLength = 8; | |
| foreach (var segment in segments) | |
| { | |
| if (segment.Length > expectedLength) | |
| { | |
| throw new FormatException($"Segment '{segment}' length {segment.Length} exceeds expected length {expectedLength}."); | |
| } | |
| } | |
| str = string.Join("", segments.Select(i => i.PadLeft(expectedLength, '0'))); | |
| } |
| } | ||
|
|
||
| // 把 Hex 形式的 str 转化为 byte[] | ||
| if (str.Length % 8 != 0) |
There was a problem hiding this comment.
nitpick: Exception message refers to 'odd number of digits' for binary conversion.
Update the exception message to specify that the string length must be a multiple of 8 for binary conversion.
Comment on lines
+11
to
+20
| [Fact] | ||
| public void ToHexString_Null() | ||
| { | ||
| var actual = BinConverter.ToString(null); | ||
| Assert.Equal(string.Empty, actual); | ||
|
|
||
| actual = BinConverter.ToString([]); | ||
| Assert.Equal(string.Empty, actual); | ||
| } | ||
|
|
There was a problem hiding this comment.
suggestion (testing): Exception test does not cover valid binary string with odd length.
Please add a test for a raw binary string with odd length, such as "0001101", to verify the exception is correctly thrown in this case.
Comment on lines
+11
to
+20
| [Fact] | ||
| public void ToHexString_Null() | ||
| { | ||
| var actual = HexConverter.ToString(null); | ||
| Assert.Equal(string.Empty, actual); | ||
|
|
||
| actual = HexConverter.ToString([]); | ||
| Assert.Equal(string.Empty, actual); | ||
| } | ||
|
|
There was a problem hiding this comment.
suggestion (testing): Exception test does not cover valid hex string with odd length and with separators.
Please add a test with a hex string containing separators that results in an odd digit count after removing separators, to verify the exception is correctly thrown in this case.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link issues
fixes #543
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add static HexConverter and BinConverter classes to convert between byte arrays and hexadecimal or binary strings, complete with validation and custom separators, and include unit tests for both converters
New Features:
Tests: