Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Given a C# string which is a set of hexadecimal numbers such as:

string HexString = "202048656c6c6f20576f726c64313233212020";

Where those hexadecimal numbers represent the ASCII text:

"  Hello World123!  "

I need to convert the HexString to a String and I have the following code which works, but looks so excessive!

    string HexStringToString(string HexString) {
        string stringValue = "";
        for (int i = 0; i < HexString.Length / 2; i++) {
            string hexChar = HexString.Substring(i * 2, 2);
            int hexValue = Convert.ToInt32(hexChar, 16);
            stringValue += Char.ConvertFromUtf32(hexValue);
        }
        return stringValue;
    }

Am I missing some elegant method?

share|improve this question
    
This method should be static as it appears that it doesn't access any class instance data. Further, it looks like an excellent candidate to be an extension method (simply change the parameter to read this string HexString after making the method static). –  Jesse C. Slicer 6 hours ago

2 Answers 2

I don't think there is a built in method. Yours is pretty good but we could make some improvements.

Parameters should be camelCase => hexString.
You should favour StringBuilder when building up strings.
You should step through the string in increments of 2 to cut down on the maths.
You should validate the argument.
You should prefer var when the type is obvious.

Result of those points:

string HexStringToString(string hexString) 
{
    if (hexString == null || (hexString.Length & 1) == 1) 
    {
        throw new ArgumentException();
    }
    var sb = new StringBuilder();
    for (var i = 0; i < hexString.Length; i += 2) {
        var hexChar = hexString.Substring(i, 2);
        sb.Append((char)Convert.ToByte(hexChar, 16));
    }
    return sb.ToString();
}
share|improve this answer
    
I really appreciate your improvements. My code is typically just reviewed by me, so I don't get much good advice! I will have to start using StringBuilder as it really is not in my repertoire (I have a C/assy background). As far as the math of i+2 vs i*2, I would "assume" the compiler sees *2 as a shift and no real multiplication. (I totally misunderstand var.) In this case the hexString is known to consist of pairs of hex digits on entry; however, to make it reusable, definitely validate. As to camelCase, I have to admit my functions, properties, methods and parameters are ALLOverthePlace! –  frog_jr 2 hours ago

The method does two different things and thus should be split in two:

  • Interpret a hex string as a sequence of bytes. You can find many possible implementations at How do you convert Byte Array to Hexadecimal String, and vice versa?.

    Yours has quadratic runtime (due to the string concatenation pattern RobH noted) and creates a new string object for each byte.

    Keeping it similar to yours:

    public static byte[] HexStringToBytes(string hexString)
    {
        if(hexString.Length % 2 != 0)
             throw new ArgumentException("hexString must have an even length", "hexString");
        var bytes = new byte[hexString.Length / 2];
        for (int i = 0; i < bytes.Length; i++)
        {
            string currentHex = hexString.Substring(i * 2, 2);
            bytes[i] = Convert.ToByte(currentHex, 16);
        }
        return bytes;
    }
    
  • Interpret the sequence of bytes as an ISO-8859-1 encoded string (the first 256 codepoints in unicode match the ISO-8859-1 single byte encoding)

    I'd use:

    Encoding.GetEncoding("ISO-8859-1").GetString(bytes)
    
share|improve this answer
    
I think this answer is better than mine - I certainly think you're right about splitting it into two methods. Good point about being explicit about the encoding as well. –  RobH 4 hours ago

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.