Skip to content

[API Proposal]: Add lowercase support for Convert.ToHexString #60393

Closed

Description

@determ1ne

Background and motivation

In some scenarios uppercase hex string is preferred, but in the others lowercase is required. See #39702 (comment).

Convert.ToHexString Methods use HexConverter.ToString internally, which supports uppercase and lowercase.

public static string ToHexString(ReadOnlySpan<byte> bytes)
{
if (bytes.Length == 0)
return string.Empty;
if (bytes.Length > int.MaxValue / 2)
throw new ArgumentOutOfRangeException(nameof(bytes), SR.ArgumentOutOfRange_InputTooLarge);
return HexConverter.ToString(bytes, HexConverter.Casing.Upper);
}

But HexConverter is an internal class, and the case transforming in HexConverter.ToString is faster than calling String.ToUpper() again, because HexConverter use ssse3 or bit operations.

Add a new method will be simple because the internal class has supported it already.

API Proposal

namespace System
{
    public static class Convert
    {
        public static string ToHexString(byte[] inArray, Casing casing = Casing.Upper);
        public static string ToHexString(byte[] inArray, int offset, int length, Casing casing = Casing.Upper);
        public static string ToHexString(ReadOnlySpan<byte> bytes, Casing casing = Casing.Upper);
    }
}

API Usage

var byteArray = GetByteArray();
var hexString = Convert.ToHexString(byteArray, Casing.Lower);
Console.WriteLine(hexString);

Risks

No response

Activity

  1. added
    api-suggestionEarly API idea and discussion, it is NOT ready for implementation
    on Oct 14, 2021
  2. ghost added
    untriagedNew issue has not been triaged by the area owner
    on Oct 14, 2021
  3. tannergooding commented on Oct 14, 2021

    @tannergooding
    Member

    I wonder if Convert.ToHexStringLower would be better. The Casing enum is more of an internal implementation detail to make a few internal things "easier" and shouldn't be publicly exposed.

    bool might be another option, but given we probably shouldn't ever have more than just Upper and Lower here so it might not be necessary.

  4. Symbai commented on Oct 14, 2021

    @Symbai

    Instead of a bool or casing enum maybe this should be improved further solving other cases as well, see #50588 Then we could get rid of all at once, instead of creating APIs for every use case, just me 2 cents?!

  5. determ1ne commented on Oct 15, 2021

    @determ1ne
    ContributorAuthor

    Instead of a bool or casing enum maybe this should be improved further solving other cases as well, see #50588 Then we could get rid of all at once, instead of creating APIs for every use case, just me 2 cents?!

    The original contributed api was more complicated, but in the end they were simplified. See #17837 (comment) . However, casing is already supported in the fundamentals.

  6. determ1ne commented on Oct 15, 2021

    @determ1ne
    ContributorAuthor

    I wonder if Convert.ToHexStringLower would be better. The Casing enum is more of an internal implementation detail to make a few internal things "easier" and shouldn't be publicly exposed.

    bool might be another option, but given we probably shouldn't ever have more than just Upper and Lower here so it might not be necessary.

    I personally prefer bool. After all it's really strange if there is no Convert.ToHexStringUpper along with Convert.ToHexStringLower.

  7. neon-sunset commented on Nov 29, 2021

    @neon-sunset
    Contributor

    Sorry for intrusion but I'd like to strongly support this issue. When implementing an API for some of the internal libraries at my current workplace, there was no option to either specify the casing or supply a mutable buffer to Convert.ToHexString(input) which forces the chaining of ToLowerInvariant() without an option to avoid allocating an output string twice. The library API itself is using bool flag to specify the case but it could've as well been either an enum or bitmask enum options if a more advanced customisation was deemed necessary.

  8. Symbai commented on Nov 29, 2021

    @Symbai

    @neon-sunset When Convert.ToHexString was approved there was a video discussion. Lower case was part of the API proposal but the .NET team decided to not take it. The argument was that its actually just a one liner and that they will keep it short and simple first as they tend to over engineer APIs. Also regarding your enum this was explicit denied outside of the video but within the API proposal thread.

  9. determ1ne commented on Dec 2, 2021

    @determ1ne
    ContributorAuthor

    @neon-sunset When Convert.ToHexString was approved there was a video discussion. Lower case was part of the API proposal but the .NET team decided to not take it. The argument was that its actually just a one liner and that they will keep it short and simple first as they tend to over engineer APIs. Also regarding your enum this was explicit denied outside of the video but within the API proposal thread.

    It's not convincing because an optional argument can keep it simple too. Plus, it's a confusing fact that runtime internals can take the advantage of the faster HexConverter.ToString, but app developers need an additional convert which is absolutely slower than the former. The inefficiency can turn into a trouble problem in web apps, finally a more hack, user-implemented method is needed to replace the runtime API method. I'm now using code fragments form tkp1n/HexMate, and it can be fast and more aggressively vectorized. The .NET team should really reconsider the problem.

  10. rcollette commented on Jan 13, 2022

    @rcollette

    After watching the video (https://www.youtube.com/watch?v=H3obznO9_uo&t=2528s) The options flag seems like the next step.

  11. Genbox commented on Jun 26, 2022

    @Genbox

    In case anyone is interested, here are the benchmark results between .NET 6 and HexMate:
    image

    It clearly shows that HexMate with the HexFormattingOptions.Lowercase flag is much faster than using ToLowerInvariant().

    Source code:

    using BenchmarkDotNet.Attributes;
    using BenchmarkDotNet.Configs;
    using HexMate;
    
    [GroupBenchmarksBy(BenchmarkLogicalGroupRule.ByCategory)]
    [CategoriesColumn]
    public class HexBenchmarks
    {
        private readonly string _hex = "f2c74bf609159f27dd89a829501ec34d6596d8b39a2cce7add73a8207088817a";
        private readonly byte[] _bytes = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32 };
    
        [BenchmarkCategory("Encode")]
        [Benchmark(Baseline = true)]
        public string HexEncodeDotnet()
        {
            return System.Convert.ToHexString(_bytes);
        }
    
        [BenchmarkCategory("Encode")]
        [Benchmark]
        public string HexEncodeDotnetToLower()
        {
            return System.Convert.ToHexString(_bytes).ToLowerInvariant();
        }
    
        [BenchmarkCategory("Decode")]
        [Benchmark(Baseline = true)]
        public byte[] HexDecodeDotnet()
        {
            return System.Convert.FromHexString(_hex);
        }
    
        [BenchmarkCategory("Encode")]
        [Benchmark]
        public string HexEncodeHexMate()
        {
            return HexMate.Convert.ToHexString(_bytes);
        }
    
        [BenchmarkCategory("Encode")]
        [Benchmark]
        public string HexEncodeHexMateToLower()
        {
            return HexMate.Convert.ToHexString(_bytes).ToLowerInvariant();
        }
    
        [BenchmarkCategory("Encode")]
        [Benchmark]
        public string HexEncodeHexMateLowerFlag()
        {
            return HexMate.Convert.ToHexString(_bytes, HexFormattingOptions.Lowercase);
        }
    
        [BenchmarkCategory("Decode")]
        [Benchmark]
        public byte[] HexDecodeHexMate()
        {
            return HexMate.Convert.FromHexString(_hex);
        }
    }
    
  12. danmoseley commented on Jun 26, 2022

    @danmoseley
    Contributor

    @tannergooding do you think this should go to API review? Perhaps with your simple proposal. There's considerable upvotes so it seems like there's a real need for something more efficient than post processing the casing.

  13. 15 remaining items

  14. bartonjs commented on Sep 21, 2023

    @bartonjs
    Member

    Video

    We discussed it, and rather than adding an enum (or bool), we feel that the parallel method group with the "Lower" suffix is better, even if slightly unbalanced.

    namespace System
    {
        public static class Convert
        {
            public static string ToHexStringLower(byte[] inArray);
            public static string ToHexStringLower(byte[] inArray, int offset, int length);
            public static string ToHexStringLower(ReadOnlySpan<byte> bytes);
            public static bool TryToHexStringLower(ReadOnlySpan<byte> source, Span<char> destination, out int charsWritten);
        }
    }
  15. added
    api-approvedAPI was approved in API review, it can be implemented
    and removed
    api-ready-for-reviewAPI is ready for review, it is NOT ready for implementation
    on Sep 21, 2023
  16. tannergooding commented on Sep 22, 2023

    @tannergooding
    Member

    Marked help wanted. Please let me know if you'd like to pick this up and contribute the new API surface.

  17. ghost added
    in-prThere is an active PR which will close this issue when it is merged
    on Sep 22, 2023
  18. determ1ne commented on Sep 22, 2023

    @determ1ne
    ContributorAuthor

    Marked help wanted. Please let me know if you'd like to pick this up and contribute the new API surface.

    Please take a look at #92483

  19. tomrus88 commented on Sep 23, 2023

    @tomrus88

    Can't believe that such a simple thing requires over 2 years to be added...

  20. crozone commented on Sep 23, 2023

    @crozone

    Video

    On the issue of supporting dashes, once we can .TryToHexStringLower() to a destination span, it'd be pretty easy to pre-allocate a larger span and shuffle the output around to achieve any required formatting, which would also require no allocation. I have personally never needed dashes but very frequently need lowercase hex. MAC addresses are one common scenario for dashes but the PhysicalAddress class already handles that.

  21. crozone commented on Sep 23, 2023

    @crozone

    Also it seems like the decision has been made, but HexFormattingOptions would have been substantially cleaner. It aligns with .ToBase64CharArray() and .ToBase64String()'s Base64FormattingOptions (which also only has a single option in it), and wouldn't add an entire extra method that also doesn't match the original method (.ToHexString() should have been called .ToHexStringUpper()).

  22. ghost removed
    in-prThere is an active PR which will close this issue when it is merged
    on Nov 20, 2023
  23. LeaFrock commented on Nov 20, 2023

    @LeaFrock
    Contributor

    Just to confirm: is this API going to be released after .NET 9 rather than 8?

    Have to wait for another one year 😅 .

  24. huoyaoyuan commented on Nov 20, 2023

    @huoyaoyuan
    Member

    The lockdown of runtime API is in August. The approval of this issue is after the lockdown of .NET 8. In fact, it's the very first approvals of .NET 9.

  25. locked and limited conversation to collaborators on Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    api-approvedAPI was approved in API review, it can be implementedarea-System.Runtimehelp wanted[up-for-grabs] Good issue for external contributors

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions