HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Parsing a string to extract values

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
valuesstringparsingextract

Problem

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 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:

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

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

Solution

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

Code Snippets

public static SensorCrcValue? TryParseCrcValue(string sensorFileLine)
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;
return new SensorCrcValue
{
    CrcValue = Convert.ToByte(crcValueField),
    IsCrcValid = crcValidField.Equals("YES")
};

Context

StackExchange Code Review Q#117062, answer score: 4

Revisions (0)

No revisions yet.