13

I wrote my own class which converts C# standard primitives into byte arrays.

Later on, I took a look at the BitConverter class source, to see how pros did it.

My code example:

public static byte[] getBytes(short value) {
    byte[] bytes = new byte[2];

    bytes[0] = (byte)(value >> 8);
    bytes[1] = (byte)value;

    return bytes;
}

BitConverter class code:

public unsafe static byte[] GetBytes(short value) 
{ 
    byte[] bytes = new byte[2];
    fixed(byte* b = bytes) 
        *((short*)b) = value;
    return bytes;
}

Why are their functions tagged as unsafe and use fixed operator?

Are such functions error prone even if they use unsafe? Should I just drop mine and use their implementation? Which is more efficient?

TaW
  • 53,122
  • 8
  • 69
  • 111
Scavs
  • 673
  • 1
  • 5
  • 16

1 Answers1

12

Yes, drop yours and use the standard library.

It is more efficient, because it copies both bytes at once by casting the byte array to a short pointer. To do this it requires fixed to allow the use of pointers, and hence the unsafe keyword.

Unsafe code is generally to be avoided in your own assemblies, as it means your code cannot be guaranteed safe, and so can only be run in fully trusted environments.

However, since Bitconverter is part of the standard .Net assemblies that are signed by Microsoft, Windows knows they are ok.

Library functions are less error prone too, because they have been battle hardened by millions of developers using them every day, whereas your function has been tested only by you.

GazTheDestroyer
  • 20,722
  • 9
  • 70
  • 103
  • Makes sense, i guess i should only create wrappers around BitConverter class, such as GetBytes from string. – Scavs Aug 15 '15 at 11:34
  • 3
    @Scavs For `String` you should use `System.Text.Encoding.GetBytes` ( https://msdn.microsoft.com/en-us/library/system.text.encoding.getbytes(v=vs.110).aspx ) , not BitConverter. – Rotem Aug 15 '15 at 11:35
  • You are right, i guess i should cut the "reinvent the wheel" thing. – Scavs Aug 15 '15 at 11:36
  • 1
    It's not necessarily bad to reinvent the wheel for educational purposes, as long as you're aware that there's something better and tested out there already (and thus you remember to use that for any "serious" purposes). – Theodoros Chatzigiannakis Aug 15 '15 at 12:37
  • 2
    The integer conversion functions in `BitConverter` are very rarely useful. If you parse files, or write communication protocols its use of native endianness causes a platform dependence. When acting on native data structure you typically have better choices available, such as the marshaller or defining a struct and casting the pointer. An top of that it's slow. – CodesInChaos Aug 15 '15 at 12:44
  • 2
    Fixing the pointer is extremely slow (it pins the object). It is by no means efficient. The binary reader/writer classes put every byte in a byte array just like the OP's method: http://referencesource.microsoft.com/#mscorlib/system/io/binarywriter.cs,258 – Vercas Aug 15 '15 at 15:35
  • 1
    _"However, since Bitconverter is part of the standard .Net assemblies that are signed by Microsoft, Windows knows they are ok"_ At least until the next patch Tuesday. – Lightness Races in Orbit Aug 15 '15 at 18:01
  • @Vercas Im interested in most efficient method. Could you please elaborate what you mean by 'Fixing the pointer is extremely slow (it pins the object)'. – Scavs Aug 16 '15 at 07:21
  • 1
    @Scavs For your pointer to the array to be valid, the object must remain in the same position in memory. The .NET garbage collector normally moves objects around to optimize its use of memory, but moving an object whose pointer you're using will invalidate that pointer and cause some serious errors. Thus, the GC needs to be told not to move the object. For primitive types, your way is the most efficient. For structs, I've got a much more efficient way: https://s.vercas.com/cerpq (it's in this commit). You ought to fix it when it's on the stack and *not on the heap* to get a major speed boost. – Vercas Aug 16 '15 at 07:43
  • 1
    @Scavs You can easily do some benchmarks to test you way against the `BitConverter`'s. The differences in performance should be significant, even when you use larger types such as `long` or `double`. – Vercas Aug 16 '15 at 07:45