Parsing strings into other data types is always potentially fraught, what with the edge cases and possible errors. This is why most languages provide some kind of helper methods that try and solve those hard problems.
C# has a number of them. One, for example, would be Int32.Parse
- it attempts to parse a string into an integer, and throws an exception when it fails. Similarly, there's an Int32.TryParse
function, which avoids throwing an exception and returns an error code instead.
Which brings us to this code, from A Barker.
public int Pieces
{
get {
int p;
Int32.TryParse(PiecesTextBox.Text, out p);
if (p != null)
{
return p;
} else {
return 0;
}
}
set {
PiecesTextBox.Text = value.ToString();
}
}
So, the first, and most fundamental problem with this code, is that it's not really a property, in the sense that the language intends it to be. It wraps a text box up as an integer, which is always going to be a fraught and problematic way to handle values in your view. Program state and view state are honestly different things, and should be compartmentalized. Also, minimize logic in properties, because when you assume something is an accessor but is actually doing complicated calculations, you're asking for problems in debugging.
That's a bad design choice. But also, this code is wrong, but will fortunately still work the same way as code that's right.
int
s, in C#, are "value types". They are not references, and cannot ever be null. if (p != null)
is a meaningless operation- it could never be false. TryParse
's behavior is that it will attempt to parse the input, and if it fails, it sets the output to the default for its type and returns an error code.
In other words, you could remove the if/else
and the behavior of the code would be unchanged: try and parse the text box, and if you fail, just treat it like a zero. I suppose I should say the behavior is almost unchanged, since the compiler doesn't optimize out that pointless condition, at least in my testing.