Parsing a string to extract values

Question

I am writing some C# that will be running on Linux and will extract values from a kernel generated system file that represents readings from a sensor.

The file contains two lines of text, and I have a method which will extract two values from the file.

The file looks like this:

$ cat /sys/bus/w1/devices/28-00000*/w1_slave
c3 01 4b 46 7f ff 0d 10 2f : crc=2f YES
c3 01 4b 46 7f ff 0d 10 2f t=28187

The method only cares about the crc=2f and YES (can also be NO) fields. The method returns a Nullable<SensorResult> type to indicate the success of parsing the fields:

• If parsing could not happen at all because the passed parameter (a string) was invalid then a null will be returned.
• If parsing was successful but the CRC was invalid (set to NO) then return a SensorResult and set appropriate fields to indicate that the CRC value is invalid.
• If parsing was successful then return a SensorResult and set fields to indicate that the CRC value is valid.
• Set the CrcValue field in SensorResult to the value extracted from the string. (If the YES/NO field is set to NO then getting the CRC value is still required.)

Because I'm parsing the string and extracting fields from specific positions, I've made a couple of enums to store these positions instead of relying on magic numbers.

Can the code be improved? If so, how? Are there any best practices I'm not following?

Parser:

    // <summary>
    // Parses the given line from the sensor file and determines if the CRC value is valid.
    // </summary>
    // <param name="sensorFileLine">The sensor file line.</param>
    // <returns>A <see cref="Nullable<SensorCrcValue>" /> that is used to indicate whether parsing the sensor data was successful.</returns>
    public static SensorCrcValue? TryParseCrcValue(string sensorFileLine)
    {
        if (string.IsNullOrWhiteSpace(sensorFileLine))
        {
            return null;
        }

        string[] initialSplitString = sensorFileLine.Split(' ');
        if (initialSplitString.Count() != (int)ParserFieldCount.CrcLineFieldCount)
        {
            return null;
        }

        string crcValueField = initialSplitString[(int)ParserFieldPosition.CrcValue].Split('=')[1];
        string crcValidField = initialSplitString[(int)ParserFieldPosition.CrcIsValid];

        SensorCrcValue sensorCrcValue;

        sensorCrcValue.CrcValue = Convert.ToByte(crcValueField);
        sensorCrcValue.IsCrcValid = false;

        if (crcValidField.Equals("YES"))
        {
            sensorCrcValue.IsCrcValid = true;
        }
        else if (crcValueField.Equals("NO"))
        {
            sensorCrcValue.IsCrcValid = false;
        }

        return sensorCrcValue;
    }

SensorResult struct:

public struct SensorCrcValue
{
    public bool IsCrcValid;
    public byte CrcValue;
}

Answer 1

public static SensorCrcValue? TryParseCrcValue(string sensorFileLine)

A common design pattern for Try[Action] methods is to return a boolean value to indicate if the action were successful or not and let the result be passed back to the caller through a by reference argument.

public static bool TryParseCrcValue(string sensorFileLine, out SensorCrcValue result)

---

SensorCrcValue sensorCrcValue;

sensorCrcValue.CrcValue = Convert.ToByte(crcValueField);
sensorCrcValue.IsCrcValid = false;

if (crcValidField.Equals("YES"))
{
    sensorCrcValue.IsCrcValid = true;
}
else if (crcValueField.Equals("NO"))
{
    sensorCrcValue.IsCrcValid = false;
}

return sensorCrcValue;

The last part of your code can be shortened since IsCrcValid is not a nullable boolean.

return new SensorCrcValue
{
    CrcValue = Convert.ToByte(crcValueField),
    IsCrcValid = crcValidField.Equals("YES")
};