Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

As the title explains, this is a series of extension methods that convert certain numeric types to and from byte-arrays, for certain actions which work better on byte-array types than numeric types.

Any and all suggestions are welcome, I am also attaching the Unit Tests.

/// <summary>
/// Provides extension methods to convert certian base types to and from a byte-array.
/// </summary>
public static class NumberByteArrayExtensions
{
    /// <summary>
    /// Converts a <code>uint</code> value to a <code>byte[]</code>.
    /// </summary>
    /// <param name="value">The <code>uint</code> value to convert.</param>
    /// <returns>A <code>byte[]</code> representing the <code>uint</code> value.</returns>
    public static byte[] ToByteArray(this uint value)
    {
        var size = 4;

        var result = new byte[size];

        for (var i = 0; i < size; i++)
        {
            var bitOffset = (size - (i + 1)) * 8;
            result[i] = (byte)((value & ((ulong)0xFF << bitOffset)) >> bitOffset);
        }

        return result;
    }

    /// <summary>
    /// Converts a <code>byte[]</code> to a <code>uint</code> value.
    /// </summary>
    /// <param name="data">The <code>byte[]</code> to convert.</param>
    /// <returns>A <code>uint</code> that represents the converted <code>byte[]</code>.</returns>
    public static uint ToUInt32(this byte[] data)
    {
        var requiredSize = 4;

        if (data.Length != requiredSize)
        {
            throw new ArgumentException($"The byte-array \"{nameof(data)}\" must be exactly {requiredSize} bytes.");
        }

        var result = 0u;

        for (var i = 0; i < requiredSize; i++)
        {
            result |= ((uint)data[i] << ((requiredSize - (i + 1)) * 8));
        }

        return result;
    }

    /// <summary>
    /// Converts an <code>int</code> value to a <code>byte[]</code>.
    /// </summary>
    /// <param name="value">The <code>int</code> to convert.</param>
    /// <returns>A <code>byte[]</code> representing the <code>int</code> value.</returns>
    public static byte[] ToByteArray(this int value)
    {
        var t = (uint)value;

        return t.ToByteArray();
    }

    /// <summary>
    /// Converts a <code>byte[]</code> to an <code>int</code> value.
    /// </summary>
    /// <param name="data">The <code>byte[]</code> to convert.</param>
    /// <returns>An <code>int</code> value representing the <code>byte[]</code>.</returns>
    public static int ToInt32(this byte[] data)
    {
        var requiredSize = 4;

        if (data.Length != requiredSize)
        {
            throw new ArgumentException($"The byte-array \"{nameof(data)}\" must be exactly {requiredSize} bytes.");
        }

        return (int)data.ToUInt32();
    }

    /// <summary>
    /// Converts a <code>ulong</code> to a <code>byte[]</code>.
    /// </summary>
    /// <param name="value">The <code>ulong</code> to convert.</param>
    /// <returns>A <code>byte[]</code> representing the <code>ulong</code>.</returns>
    public static byte[] ToByteArray(this ulong value)
    {
        var size = 8;

        var result = new byte[size];

        for (var i = 0; i < size; i++)
        {
            var bitOffset = (size - (i + 1)) * 8;
            result[i] = (byte)((value & ((ulong)0xFF << bitOffset)) >> bitOffset);
        }

        return result;
    }

    /// <summary>
    /// Converts a <code>byte[]</code> to a <code>ulong</code>.
    /// </summary>
    /// <param name="data">The <code>byte[]</code> to convert.</param>
    /// <returns>A <code>ulong</code> reprented by the <code>byte[]</code>.</returns>
    public static ulong ToUInt64(this byte[] data)
    {
        var requiredSize = 8;

        if (data.Length != requiredSize)
        {
            throw new ArgumentException($"The byte-array \"{nameof(data)}\" must be exactly {requiredSize} bytes.");
        }

        var result = 0ul;

        for (var i = 0; i < requiredSize; i++)
        {
            result |= ((ulong)data[i] << ((requiredSize - (i + 1)) * 8));
        }

        return result;
    }

    /// <summary>
    /// Converts a <code>long</code> value to a <code>byte[]</code>.
    /// </summary>
    /// <param name="value">The <code>long</code> value to convert.</param>
    /// <returns>A <code>byte[]</code> representing the <code>long</code> value.</returns>
    public static byte[] ToByteArray(this long value)
    {
        var t = (ulong)value;

        return t.ToByteArray();
    }

    /// <summary>
    /// Converts a <code>byte[]</code> to a <code>long</code> value.
    /// </summary>
    /// <param name="data">The <code>byte[]</code> to convert.</param>
    /// <returns>A <code>long</code> value represented by the <code>byte[]</code>.</returns>
    public static long ToInt64(this byte[] data)
    {
        var requiredSize = 8;

        if (data.Length != requiredSize)
        {
            throw new ArgumentException($"The byte-array \"{nameof(data)}\" must be exactly {requiredSize} bytes.");
        }

        return (long)data.ToUInt64();
    }
}

Unit Tests:

[TestClass]
public class NumberByteArrayExtensionsTests
{
    [TestMethod, TestCategory("Number Byte-Array Extensions Tests")]
    public void UIntToByteArray_0xFF007FBF()
    {
        var sourceNumber = 0xFF007FBFu;
        var resultArray = sourceNumber.ToByteArray();
        var expectedResult = new byte[] { 0xFF, 0x00, 0x7F, 0xBF };

        CollectionAssert.AreEqual(expectedResult, resultArray);
    }

    [TestMethod, TestCategory("Number Byte-Array Extensions Tests")]
    public void ByteArrayToUInt_0xFF_0x00_0x7F_0xBF()
    {
        var sourceArray = new byte[] { 0xFF, 0x00, 0x7F, 0xBF };
        var resultNumber = sourceArray.ToUInt32();
        var expectedNumber = 0xFF007FBFu;

        Assert.AreEqual(expectedNumber, resultNumber);
    }

    [TestMethod, TestCategory("Number Byte-Array Extensions Tests")]
    public void ULongToByteArray_0xFFAF0FCF4F007FBF()
    {
        var sourceNumber = 0xFFAF0FCF4F007FBFu;
        var resultArray = sourceNumber.ToByteArray();
        var expectedResult = new byte[] { 0xFF, 0xAF, 0x0F, 0xCF, 0x4F, 0x00, 0x7F, 0xBF };

        CollectionAssert.AreEqual(expectedResult, resultArray);
    }

    [TestMethod, TestCategory("Number Byte-Array Extensions Tests")]
    public void ByteArrayToULong_0xFF_0xAF_0x0F_0xCF_0x4F_0x00_0x7F_0xBF()
    {
        var sourceArray = new byte[] { 0xFF, 0xAF, 0x0F, 0xCF, 0x4F, 0x00, 0x7F, 0xBF };
        var resultNumber = sourceArray.ToUInt64();
        var expectedNumber = 0xFFAF0FCF4F007FBFu;

        Assert.AreEqual(expectedNumber, resultNumber);
    }

    [TestMethod, TestCategory("Number Byte-Array Extensions Tests")]
    public void IntToByteArray_0x7F00FFBF()
    {
        var sourceNumber = 0x7F00FFBF;
        var resultArray = sourceNumber.ToByteArray();
        var expectedResult = new byte[] { 0x7F, 0x00, 0xFF, 0xBF };

        CollectionAssert.AreEqual(expectedResult, resultArray);
    }

    [TestMethod, TestCategory("Number Byte-Array Extensions Tests")]
    public void ByteArrayToInt_0x7F_0x00_0xFF_0xBF()
    {
        var sourceArray = new byte[] { 0x7F, 0x00, 0xFF, 0xBF };
        var resultNumber = sourceArray.ToInt32();
        var expectedNumber = 0x7F00FFBF;

        Assert.AreEqual(expectedNumber, resultNumber);
    }

    [TestMethod, TestCategory("Number Byte-Array Extensions Tests")]
    public void LongToByteArray_0x7FAF0FCF4F00FFBF()
    {
        var sourceNumber = 0x7FAF0FCF4F00FFBF;
        var resultArray = sourceNumber.ToByteArray();
        var expectedResult = new byte[] { 0x7F, 0xAF, 0x0F, 0xCF, 0x4F, 0x00, 0xFF, 0xBF };

        CollectionAssert.AreEqual(expectedResult, resultArray);
    }

    [TestMethod, TestCategory("Number Byte-Array Extensions Tests")]
    public void ByteArrayToLong_0x7F_0xAF_0x0F_0xCF_0x4F_0x00_0xFF_0xBF()
    {
        var sourceArray = new byte[] { 0x7F, 0xAF, 0x0F, 0xCF, 0x4F, 0x00, 0xFF, 0xBF };
        var resultNumber = sourceArray.ToInt64();
        var expectedNumber = 0x7FAF0FCF4F00FFBF;

        Assert.AreEqual(expectedNumber, resultNumber);
    }
}

Feel free to review everything (including the tests).

share|improve this question

6 Answers 6

First of all, I agree with others, you could replace lines

var size = 4;
var requiredSize = 8;

with

const int size = sizeof(int);
const int requiredSize = sizeof(long);

This removes magic values from the code.


Secondly, your code is hard to understand. For instance:

result[i] = (byte)((value & ((ulong)0xFF << bitOffset)) >> bitOffset);

My suggestion is to use structs with [StructLayout(LayoutKind.Explicit)] attribute:

[StructLayout(LayoutKind.Explicit)]
public struct UnionInt
{
    [FieldOffset(0)]
    public readonly int Value;
    [FieldOffset(0)]
    private readonly byte byte0;
    [FieldOffset(1)]
    private readonly byte byte1;
    [FieldOffset(2)]
    private readonly byte byte2;
    [FieldOffset(3)]
    private readonly byte byte3;

    public byte[] Bytes
    {
        get { return new[] { byte0, byte1, byte2, byte3 }; }
        // Or { byte3, byte2, byte1, byte0 } to change endianess
    }

    public UnionInt(int value)
    {
        byte0 = byte1 = byte2 = byte3 = 0;
        Value = value;
    }
}
share|improve this answer
    
I like the struct trick ;) – t3chb0t 22 hours ago
1  
Rather than using structs, one could instead use BitConverter.GetBytes. – Kyle 21 hours ago
1  
@Kyle you could put that as an answer, and perhaps provide an example or two. – Dan Lyons 21 hours ago
3  
@Kyle I know. Pay attention on the reinventing-the-wheel tag. – Dmitry 21 hours ago
    
Just a remark regarding the use of StructLayout. This doesn't take endianess into consideration, and will result in the bytes being flipped around on those systems. This may not be an issue when you're locked in on a platform, but for cross-platform development it'll cause problems with interoperability. (Most notably, with the use of serialization.) – Aidiakapi 19 hours ago

I agree with @t3chb0t, looks pretty good.

Except...

    var resultNumber = sourceArray.ToInt32();
    var expectedNumber = 0x7F00FFBF;
    var resultArray = sourceNumber.ToByteArray();
    var expectedResult = new byte[] { 0x7F, 0xAF, 0x0F, 0xCF, 0x4F, 0x00, 0xFF, 0xBF };

I find ToIn32 and ToByteArray already make it clear that we're getting an Int32 and byte[]. why not just name them for what they're used?

    var actual = sourceArray.ToInt32();
    var expected = 0x7F00FFBF;
    var actual = sourceNumber.ToByteArray();
    var expected = new byte[] { 0x7F, 0xAF, 0x0F, 0xCF, 0x4F, 0x00, 0xFF, 0xBF };

Funny, as I renamed these identifiers something hit me: I consider defining the expected value as part of the "Arrange" step of an "Arrange-Act-Assert" unit test. Therefore, I'd rather have them like this:

    var expected = 0x7F00FFBF;
    var actual = sourceArray.ToInt32();
    var expected = new byte[] { 0x7F, 0xAF, 0x0F, 0xCF, 0x4F, 0x00, 0xFF, 0xBF };
    var actual = sourceNumber.ToByteArray();

This respects the order of the parameters of the Assert method you're calling, too: Assert.AreEqual(expected, actual);.

share|improve this answer
1  
I always forget about the order of parameters, thanks for the tip on that! I'll have to start rearranging my Unit Tests to follow that style. – EBrown 22 hours ago

I don't find many things to complain about :-) but this bothers me...

I would change the name of those variables:

var requiredSize = 4;
var size = 4;

to something like

var intSize = sizeof(int);

or

var numOfInt32Bytes = sizeof(int);

It might be the required size but what size is it actually? The code doesn't tell us this.


This method and it's t are very confusing:

public static byte[] ToByteArray(this int value)
{
    var t = (uint)value;

    return t.ToByteArray();
}

If you cast the int into a unit it would be a big surprise to the user if the resulting array looses it's sign. I think the parameter should either be a uint or the conversion needs to take a negative value into consideration.

share|improve this answer
1  
Funny I was just looking at that t... nice answer :-) – Mat's Mug 22 hours ago
1  
@t3chb0t The array doesn't lose it's sign, the initial value might but the bit is preserved. – EBrown 21 hours ago
    
@EBrown oh, you're right. I've just tested with BitConverter it and indeed, the sign is there. I've learned something new agian ;-) – t3chb0t 21 hours ago

I got 2 comments, so this'll be a small answer.

You have a potential NullReferenceException. All your methods receiving a byte[] don't check for null! So either you should return 0, or you should throw an ArgumentNullException. (You could argue this might not happen a lot since you're using extension methods, but ((byte[])null).ToInt32() compiles very well ;))

Second, your unit tests should be self explanatory. I think the code within the tests is good, very explicit. But the names of your test shouldn't be specific to the unit tested, but to what is tested within the unit. From a book I read (sorry, I don't have the reference :p You can decide whether or not you like this idea) this is a good way to write tests :

MethodName_Context_Result()

You got the good start, and some methods don't go hand in hand with that "convention". But, some do!

Ex :

  • ToInt32_NullArray_ArgumentNullException()
  • ToInt32_ArrayRepresents42_42()
  • etc...

I agree that for the second example, the convention might not be good. But I don't feel like writing the hexadecimal value in the test name is anyhow useful!

PS : There's no need to put it in my answer, everyone talked about it, but I want to stress that you should change

var requiredSize = 8;

TO

const int requiredSize = sizeof(long);

And even, I don't think using the const is useful. sizeof(long) is the most explicit piece of code you can have.

EX :

/// <summary>
/// Converts a <code>byte[]</code> to an <code>int</code> value.
/// </summary>
/// <param name="data">The <code>byte[]</code> to convert.</param>
/// <returns>An <code>int</code> value representing the <code>byte[]</code>.</returns>
public static int ToInt32(this byte[] data)
{
    if(data == null)
    {
        throw new ArgumentNullException("wth dude");
    }

    if (data.Length != sizeof(int))
    {
        throw new ArgumentException($"The byte-array \"{nameof(data)}\" must be exactly {requiredSize} bytes.");
    }

    return (int)data.ToUInt32();
}
share|improve this answer
1  
I like how this answer starts off as "this will be a short answer"... and then goes on and on and on ;-) ...I know the feeling! – Mat's Mug 18 hours ago
    
Yeah I didn't think I had much content.. Ahah :p – TopinFrassi 18 hours ago

Type aliases int uint are fixed to 32 bits. There is no point in using sizeof for correctness, but you may prefer it for readability.

Methods use loops where there are only 4 iterations. Int-to-byte explicit convertion is unchecked, meaning does not cause exceptions due to overflow. Shift has lower priority than addition, causing more parentheses. Consider simpler code:

public static byte[] ToByteArray(this uint value)
{
    return new byte[] { (byte)(value >> 24), (byte)(value >> 16),
        (byte)(value >> 8), (byte)(value) };
}

public static uint ToUInt32(this byte[] data)
{
    if (data.Length != 4)
        throw new ArgumentException("Byte array must be exactly 4 bytes to be convertible to uint.");

    return ((((((uint)data[0] << 8) + data [1]) << 8) + data [2]) << 8) + data [3];
}

Signed int/long methods check length of array twice. Same check is performed by underlying method after a cast. You should remove duplicated checks for performance.

public static byte[] ToByteArray(this int value)
{
    return ToByteArray((uint)value);
}

Documentation should explicitly state that BigEndian encoding is used.

There is a built-in .NET implementation in BitConverter class.

share|improve this answer

You have some small code duplication there regarding

(size - (i + 1)) * 8 

which could be extracted to a separate private (extension) method like so

private static int CalculateOffset(this int value, int index)
{
    return (value - index - 1 ) * 8;
}  

and be called like

public static byte[] ToByteArray(this uint value)
{
    var size = 4;

    var result = new byte[size];

    for (var i = 0; i < size; i++)
    {
        var bitOffset = size.CalculateOffset(i);
        result[i] = (byte)((value & ((ulong)0xFF << bitOffset)) >> bitOffset);
    }

    return result;
}  

public static uint ToUInt32(this byte[] data)
{
    var requiredSize = 4;

    if (data.Length != requiredSize)
    {
        throw new ArgumentException($"The byte-array \"{nameof(data)}\" must be exactly {requiredSize} bytes.");
    }

    var result = 0u;

    for (var i = 0; i < requiredSize; i++)
    {
        result |= ((uint)data[i] << requiredSize.CalculateOffset(i);
    }

    return result;
}  

etc.

Usually I would suggest like @TopinFrassin that you should do a null check and throw an ArgumentNullException but this exception is thrown anyway and there isn't any leading processing taking place. That simple variable initializing ( var requiredSize = 8; ) won't do any harm.

For the tests I tend to use class level variables to hold content which is repeatedly used like this var expectedResult = new byte[] { 0xFF, 0x00, 0x7F, 0xBF };.

Another point is that your tests won't test for edge cases like the mentioned passing of null to e.g public static uint ToUInt32(this byte[] data). Another test should pass an arry which hasn't the needed size.

You should also test that the title of your question "Numbers to byte-arrays and back" is valid by adding tests which for instance taking a ulong convert it to a byte array and back and assert that it is the same ulong.

I like it if I see in the documentation what type and in which situation the code will throw for instance an ArgumentException. You should consider to add this to the documentation.

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.