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.

What I am creating is essentially a binary clock face with touch input for setting values as well as displaying - so I need to convert both ways between an int and a binary sequence in a BitArray.

I cobbled this together (borrow and modifying a bit from a couple different examples I found):

    public class BinaryConverter
    {
        public static BitArray ToBinary(int numeral)
        {
            BitArray binary = new BitArray(new int[] { numeral });
            bool[] bits = new bool[binary.Count];
            binary.CopyTo(bits, 0);
            return binary;
        }

        public static int ToNumeral(BitArray binary, int length)
        {
            int numeral = 0;
            for (int i = 0; i < length; i++)
            {
                if (binary[i])
                {
                    numeral = numeral | (((int)1) << (length - 1 - i));
                }
            }
            return numeral;
        }
    }

It isn't especially verbose, but in my head before I started it should have been a couple of lines per method, perhaps better leveraging some .NET classes like System.Convert (though, I can't quite see how to do that). Is there a cleaner way to do this? Do you have any other suggestions for improvement?

share|improve this question
add comment

1 Answer

up vote 8 down vote accepted

You've made it much more complicated than necessary.

The conversion to a BitArray needlessly copies the values to the bool array bits. You could instead use that on the conversion back to int.

public static class BinaryConverter
{
    public static BitArray ToBinary(this int numeral)
    {
        return new BitArray(new[] { numeral });
    }

    public static int ToNumeral(this BitArray binary)
    {
        if (binary == null)
            throw new ArgumentNullException("binary");
        if (binary.Length > 32)
            throw new ArgumentException("must be at most 32 bits long");

        var result = new int[1];
        binary.CopyTo(result, 0);
        return result[0];
    }
}
share|improve this answer
add comment

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.