Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I'm writing classes to decode and persist Message's sent via TCP. Each message contains many DataField's. I have a simple class DataField class that holds some meta data, including a

private byte[] _content;
public byte[] Content
{
    get
    {
        return _content;
    }
    set
    {
        _content = value;
    }
}

I then inherit DataField to do the customised fields as required

    public GPSData(byte[] content)
    {
        this.FieldId = 0;
        this.FieldName = "GPS Data";
        this.FixedFieldLength = 21;
        this.RecordLength = 1;
        this.LengthType = DataFieldLengthType.Fixed;
        this.ActualFieldLength = content.Length;
        this.Content = content;

        if (ActualFieldLength != FixedFieldLength)
        {
            string message = this.FieldName + " is of Data Type " + LengthType.ToString() + ".  The ActualFieldLength passed in must match the FixedFieldLength";
            throw new ArgumentOutOfRangeException(message);
        }

        int i = 0;
        GPSUTCDateTime = BitConverter.ToUInt32(content, i);
        i += sizeof(UInt32);
        Latitude = BitConverter.ToInt32(content, i);
        i += sizeof(Int32);
        Longitude = BitConverter.ToInt32(content, i);
        i += sizeof(Int32);
        Altitude = BitConverter.ToInt16(content, i);
        i += sizeof(Int16);
        TwoDGroundSpeed = BitConverter.ToUInt16(content, i);
        i += sizeof(UInt16);
        SpeedAccuracyEstimate = content[i];
        i += sizeof(byte);
        TwoDHeading = content[i];
        i += sizeof(byte);
        PDOP = content[i];
        i += sizeof(byte);
        PositionAccuracyEstimate = content[i];
        i += sizeof(byte);
        GPSStatusFlags = content[i];
    }
}

Is this.Content = content; correct o ensure the array is assigned properly?

Should I perhaps use content.CopyTo(Content, 0);

Or is there another more correct option I should use.

Mostly worried because I don't see a new declaration in the class for the array to be allocated.

share|improve this question
    
Please show your real, working code. Placeholders like A, X, Y, and magic are not suitable for Code Review. – 200_success Dec 6 '16 at 0:37
    
Yes, it assigns the content passed in to the Content property, however be aware that arrays in C# are reference types, not value types like the elements. This means that you can change the data outside the class and the value will reflect in the class. If you want to copy the array, the second choice would be appropriate (Array.Copy) or the faster Buffer.BlockCopy. – Ron Beyer Dec 6 '16 at 15:04
    
Any of a class' constructors can be used in a new declaration. If the only constructor takes a parameter, then the only way to create a new instance is by passing that parameter. Also what does i represent? – tinstaafl Dec 6 '16 at 20:18
    
i is an index into the byte[] content It's used to keep track of where the next field is extracted from. Its then updated with how much data was taken and moved along.I'm working from a spec have a spec that details all the current DataField types. – Hecatonchires Dec 8 '16 at 1:37
    
@RonBeyer if you want to post as answer I'll accept. I'll change all the Constructors to use Array.Copy just in case. – Hecatonchires Dec 8 '16 at 1:39

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Browse other questions tagged or ask your own question.