11
\$\begingroup\$

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).

\$\endgroup\$
7
\$\begingroup\$

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);.

\$\endgroup\$
  • 2
    \$\begingroup\$ 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. \$\endgroup\$ – 410_Gone Oct 23 '15 at 17:20
12
\$\begingroup\$

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;
    }
}
\$\endgroup\$
  • \$\begingroup\$ I like the struct trick ;) \$\endgroup\$ – t3chb0t Oct 23 '15 at 17:20
  • 1
    \$\begingroup\$ Rather than using structs, one could instead use BitConverter.GetBytes. \$\endgroup\$ – Kyle Oct 23 '15 at 18:13
  • 1
    \$\begingroup\$ @Kyle you could put that as an answer, and perhaps provide an example or two. \$\endgroup\$ – Dan Lyons Oct 23 '15 at 18:14
  • 4
    \$\begingroup\$ @Kyle I know. Pay attention on the reinventing-the-wheel tag. \$\endgroup\$ – Dmitry Oct 23 '15 at 18:15
  • 4
    \$\begingroup\$ 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.) \$\endgroup\$ – Aidiakapi Oct 23 '15 at 19:47
10
\$\begingroup\$

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.

\$\endgroup\$
  • 1
    \$\begingroup\$ Funny I was just looking at that t... nice answer :-) \$\endgroup\$ – Mathieu Guindon Oct 23 '15 at 17:11
  • 2
    \$\begingroup\$ @t3chb0t The array doesn't lose it's sign, the initial value might but the bit is preserved. \$\endgroup\$ – 410_Gone Oct 23 '15 at 18:10
  • 1
    \$\begingroup\$ @EBrown oh, you're right. I've just tested with BitConverter it and indeed, the sign is there. I've learned something new agian ;-) \$\endgroup\$ – t3chb0t Oct 23 '15 at 18:14
6
\$\begingroup\$

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("data");
    }

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

    return (int)data.ToUInt32();
}
\$\endgroup\$
  • 4
    \$\begingroup\$ 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! \$\endgroup\$ – Mathieu Guindon Oct 23 '15 at 21:00
  • \$\begingroup\$ Yeah I didn't think I had much content.. Ahah :p \$\endgroup\$ – IEatBagels Oct 23 '15 at 21:07
  • \$\begingroup\$ NullReferenceException is guaranteed to happen. Not because of extension method, as ToInt32(null) can also be called, but when accessing indexer, data[...] throws when data is null. Interpreting null as zero is not a common practice, but good you put that option on the table. \$\endgroup\$ – ArekBulski Oct 25 '15 at 11:11
6
\$\begingroup\$

Types byte int uint long ulong are fixed, set in stone. byte will have 8 bits just like int will always have 32 bits. They do not depend on architecture. There is no point in using sizeof for correctness, but you may prefer it for readability.

Please note however that literals (and possibly constants) can be used by compiler to optimize the code. You are assigning 4 to a variable, not even a constant, which does not help the compiler. Either way, you should consider explicit loop unrolling for better performance.

const int size = sizeof(uint); // Good
int size = sizeof(uint); // Bad

Methods use loops where there are only 4 iterations and iteration is very lightweight. Optimization called loop unrolling has direct application here. Code below is a form of that.

Consider simpler code as below. Uint is fixed to being 4 bytes, so byte array initializer has explicitly 4 elements. Also byte is fixed to being 8 bits, so argument is shifted and sliced off one byte (eight bits) at a time. Casting (byte)uint merely chops off least significant byte, no need for bitwise-and 0xFF.

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

In this code, array length is check directly against uint size, which is always 4 bytes. Individual bytes are then merged (added) and shifted into their respective position within the uint.

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.

Extension methods allow but not require argument to be on left side of the member access (dot) operator. This simply allows a clean form like below.

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, however their implementation switches automatically between LittleEndian/BigEndian based on architecture.

Edit: For those complaining about magic numbers, look at http://referencesource.microsoft.com/#mscorlib/system/bitconverter.cs

\$\endgroup\$
  • 2
    \$\begingroup\$ Your code contains too many magic numbers for my taste. \$\endgroup\$ – guntbert Oct 24 '15 at 19:39
  • \$\begingroup\$ Does that fact that a byte has 8 bits is also a magic number to you? Sizes of these types are fixed in stone. \$\endgroup\$ – ArekBulski Oct 24 '15 at 19:55
  • \$\begingroup\$ Well, I would prefer (a) variable(s) with appropriate names - at least instead of 8. \$\endgroup\$ – guntbert Oct 24 '15 at 20:02
  • 1
    \$\begingroup\$ @guntbert me either. You start with obivous numbers and end by not writing any comments because at the time of programming everything is so simple that why waste the keyboard for typing too much ;-( \$\endgroup\$ – t3chb0t Oct 24 '15 at 20:14
  • \$\begingroup\$ @t3chb0t Sorry, I focused on performance rather than descriptiveness. I reedited the answer just now, and explained some things in more detail. \$\endgroup\$ – ArekBulski Oct 24 '15 at 21:29
4
\$\begingroup\$

I'm just going to toss my two cents in here about your unit tests.

  1. Agreed with what others have said about not putting actual values in your method names. Personally, I like Class_Method_Case_Result names. Yeah, they're long, but useful in projects of any size.
  2. There's no reason to repeat the category string all over the place.

    [TestMethod, TestCategory("Number Byte-Array Extensions")]
    

Minimally, create a class level constant.

const string ExtensionsCategory = "Number Byte-Array Extensions";

[TestMethod, TestCategory(ExtensionsCategory)]
public void SomeTestMethod()
{
    //...

However, in order for these categories to be useful in a larger project, I would create a static class that defines all of them. Remember, tests are real code too. Some of the "rules" are different, but most of the same SOLID principles apply to our tests as production code. If we can reduce duplication and make our tests easier to write & maintain, we should.

One last note on your category: I like that you're using them, but this one feels a bit useless. It doesn't provide any additional information. We could already execute the tests by class, if we wished.

I tend to use this attribute to categorize my tests in other ways. What I end up doing most often is separating my unit tests from my integration tests. For a mid size project, I often have both categories in the same test project, but I obviously don't want to run my integration tests all the time. Using this attribute allows me to separate that out on my CI server. So, that static class I mentioned could look something like this.

public static class TestCategories
{
     public const string Unit = "Unit";
     public const string Integration = "Integration";
     //...
}

Of course, use whatever categories make sense for your project and workflow.

\$\endgroup\$
  • 1
    \$\begingroup\$ As a side note, I love it when there are so many answers addressing different parts of the code like this. It keeps things interesting. =) \$\endgroup\$ – RubberDuck Oct 25 '15 at 11:09
  • 1
    \$\begingroup\$ It makes it awful hard to choose a "best answer" though. :) \$\endgroup\$ – 410_Gone Oct 25 '15 at 16:03
3
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ Good point about testing whether converting back and forth yields the same number. This can be easily automated in a loop for a bigger set of numbers. \$\endgroup\$ – ArekBulski Oct 25 '15 at 11:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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