current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 2 down vote favorite

Another journey down the difficult path! I'd like to know if there's anything I haven't accounted for, if this can be significantly improved, etc.

(Just as a side-note, I know that some of the formatting is a bit off; some places are spaced out a bit to make it more visually appealing.)

using System;

namespace Data
{
    public class BinaryHelper
    {
        public static string ByteToHexString (byte[] bytes)
        {
            char[] c = new char[bytes.Length * 2];
            byte b;

            for (int i = 0; i < bytes.Length; i++) {
                b = ((byte)(bytes [i] >> 4));
                c [i * 2] = (char)(b > 9 ? b + 0x37 : b + 0x30);
                b = ((byte)(bytes [i] & 0xF));
                c [i * 2 + 1] = (char)(b > 9 ? b + 0x37 : b + 0x30);
            }

            return new string (c);
        }

        public static byte[] HexStringToByte (string hex)
        {
            int len = hex.Length;
            byte[] bytes = new byte[len / 2];

            for (int i = 0; len > i; i += 2) {
                bytes [i / 2] = Convert.ToByte (hex.Substring (i, 2), 16);
            }

            return bytes;
        }
    }
}
share|improve this question
up vote 5 down vote accepted

Ask yourself "how can I make the meaning and intention of the code more clear?"

Consider making small helper methods rather than duplicating code:

static byte HighNibble(byte b) => (byte)(b >> 4);
static byte LowNibble(byte b) => (byte)(b & 0xF);
static char NibbleToHex => ... etc.

And now instead of

    b = ((byte)(bytes [i] >> 4));
    c [i * 2] = (char)(b > 9 ? b + 0x37 : b + 0x30)
    b = ((byte)(bytes [i] & 0xF));
    c [i * 2 + 1] = (char)(b > 9 ? b + 0x37 : b + 0x30);

Which is overwhelming in the sheer amount of punctuation, we have:

 c [i * 2] = NibbleToHex(HighNibble(bytes[i])); 
 c [i * 2 + 1] = NibbleToHex(LowNibble(bytes[i]));

So much easier to read and understand.

share|improve this answer
    
Great tip, thanks a lot! – T145 Sep 24 '16 at 3:28
up vote 2 down vote

I found the magic numbers in this line less than obvious:

c [i * 2] = (char)(b > 9 ? b + 0x37 : b + 0x30);

0x30 is clearly '0', however 0x37 is well... '7'. I actually find this more expressive, even though it's longer:

c[i * 2] = (char)(b > 9 ? b + 'A' - 10: b + '0');

If you're interested you can also find a rather more concise version of ByteToArrayString which makes use of BitConverter to perform the translation in this answer. It essentially becomes a one-liner:

return BitConverter.ToString(bytes).Replace("-","");
share|improve this answer

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.