What is the Proper approach for Constructing a PhysicalAddress object from Byte Array

Question

I'm trying to understand what the correct approach for a constructor that accepts a Byte Array with regard to how it stores it's data (specifically with PhysicalAddress)

I have an array of 6 bytes (theAddress) that is constructed once.

I have a source array of 18bytes (theAddresses) that is loaded from a TCP Connection.

I then copy the 6bytes from theAddress+offset into theAddress and construct the PhysicalAddress from it.

Problem is that the PhysicalAddress just stores the Reference to the array that was passed in. Therefore if you subsequently check the addresses they only ever point to the last address that was copied in.

When I took a look inside the PhysicalAddress with reflector it's easy to see what's going on.

public PhysicalAddress(byte[] address)
{
    this.changed = true;
    this.address = address;
}

Now I know this can be solved by creating theAddress array on each pass, but I wanted to find out what really is the best practice for this.

1. Should the constructor of an object that accepts a byte array create it's own private Variable for holding the data and copy it from the original
2. Should it just hold the reference to what was passed in.
3. Should I just created theAddress on each pass in the loop

Answer 1

I'd say in general the best practice with no other specific design considerations to consider is definitely (1) the constructor of an object that accepts a byte array should create its own private variable for holding the data and copy it from the original.

Answer 2

It sounds to me like your constructor should look like this:

public PhysicalAddress(byte[] source, int offset)
{
    // Validate arguments here :)
    this.changed = true;
    this.address = new byte[6];
    Buffer.BlockCopy(source, offset, address, 0, 6);
}

That way you can just do the copy once from the original 18 byte array.

Taking a defensive copy sounds like the right approach here - but you only need to do it once. It makes sense to do that within the constructor itself rather than forcing PhysicalAddress to trust that whatever code is calling it won't change the array afterwards.