• my name (unregistered)

    I may be wrong, but TryParse returns a bool, if it's false the parse did not work

  • Jaloopa (unregistered) in reply to my name

    The code is using the out parameter and ignoring the return value. The more standard way to do it would indeed be if (Int.TryParse(PiecesTextBox.Text, out p))...

  • (nodebb)

    since the compiler doesn't optimize out that pointless condition

    Because C# is a JIT-based language, the compiler eschews basically any optimization in favor of letting the runtime handle it. However, it does optimize out the condition, at least on newer versions of the compiler: https://sharplab.io/#v2:CYLg1APgAgzABFATHAKgUwM4BcDCAbAQwwwFgAoAb3LhoXgEsA7LVTLAWTSwAsB7YABRQAjAAY4TAA4BXLAEpqtKmVqqJzAHQoATgE8ACgW0Y0AqbIA0cXrPUttmaXnkBuRWokAzOAIcYnLACEALxwjE54CioecMoxalAA7HB+AW7RHgC+cGh4JrHu8QjJounxmYUVZJlAA=

  • (nodebb)

    There's a lot of horribleness in this code, so let's start from the top.

    Ironically implementing getters/setters that way are strictly not an anti-pattern. For example implementing code in setters with side effects (so influencing or being influenced by other members) would be an anti-pattern, code that is unreasonable or unpredictable long and of course code that can cause race conditions or deadlocks.

    Now a text box is usually an observable object, so there could be side effects, but that is not clear from the code because we don't see how the text box is actually defined. So let's give it the benefit of a doubt, otherwise see above.

    Not Int32 is horrible, the naming guide lines for C# dictate that keywords have to be used over types. It's that simple and the compile will throw a naming convention warning.

    Because an int is not a reference type or a nullable value type the condition with the null check will also result in a warning. Now the "correct code" ignoring the problem mentioned above looks like this BTW for those unfamiliar with C#:

    public int Pieces
{
	get => int.TryParse(PiecesTextBox.Text, out var p) ? p : 0;
	set => PiecesTextBox.Text = value.ToString();
}

    Addendum 2023-11-27 09:03: Cool, I see there are some random letters missing in my post. Well, I let you gals 'n' guys figure out what they were - at least the code is correct :-)

  • (nodebb) in reply to Jaloopa

    This is incorrect.

    In fact the try-parse pattern demands to check the return value because there's no mandatory value which has to be returned when it's false.

    https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/exceptions-and-performance

  • Chronomium (unregistered)

    this code is wrong, but will fortunately still work the same way as code that's right.

    I would call this unfortunate, actually.

  • Naomi (unregistered)

    This sort of thing is why I've come to love result types. Under the model seen here, the library author has to decide how to represent failures, and if they want to let the caller decide, they have to provide multiple functions. In languages that support result types, that's not an issue.

  • Naomi (unregistered) in reply to MaxiTB

    What do you think is incorrect about what they said?

  • TheCPUWizard (unregistered) in reply to MaxiTB

    @MaxiTB - "Now a text box is usually an observable object, so there could be side effects" ... Yes, but setting the property on the textbox itself could have the same side effects.... game, point, match...

    Seriously, the days of "innocuous properties" have long passed. We can discuss (but you are buying the beer) if this is a good or bad thing or a cheeseburger; but it is factually supported by massive amounts of code.

  • (nodebb) in reply to TheCPUWizard

    "Yes, but setting the property on the textbox itself could have the same side effects.... game, point, match..." I'm confused, that is exactly what I wrote...

    However keep in mind, they are both in the same scope, because both a members of the form.

  • (nodebb)

    It has to be noted that the p != nullcondition is a big fat warning since C# 1, but who cares about those warnings, amirite?!!

  • eluvatar (unregistered)

    Honestly, this is the best code I've ever seen on this site. I wouldn't be surprised if a junior wrote it, and I wouldn't even be that upset. It's not that bad.

  • (nodebb)

    I wonder if that source file started with a #nowarn pragma to disable all warnings.

    Because you know, these things are pointless, right? ... Obviously if they were important, they would be errors ;)

  • (nodebb) in reply to Ralf

    Obviously if they were important, they would be errors ;)

    -Werror is your friend... Of course, it's one of those stern and unyielding friends that everyone hates, like const in C and C++, but it's also the kind of friend that will stick by you and continue helping you no matter how much you hate it.

  • IRTFM (unregistered) in reply to MaxiTB

    The linked page doesn't actually say what you inferred, and if you look at the pattern as implemented by Microsoft the out parameter is supposed to return the type's default value on a failed return, not just some unknown value. See the docs for that method.

    While I would personally use a ternary (as suggested by others) for readability, these are functionally the same:

    int.TryParse(stringValue, out int value) ? value : default(int); // or 0 or default depending on your preference
int.TryParse(stringValue, out int value); return value;

    The latter is bad only because it assumes the reader knows that value will be valid or default. You only really need to check the result if you want a value other than the default on a failure, or if you otherwise need to distinguish invalid input returning 0 from "0".

  • (nodebb)

    @ MaxiTB Ref

    This is incorrect.

    In fact the try-parse pattern demands to check the return value because there's no mandatory value which has to be returned when it's false.

    https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/exceptions-and-performance

    This is half incorrect. The Framework's [BuiltinValueType].TryParse() methods all are documented to return the default value of the valuetype they parse towards. So if you're happy with that behavior then there is no actual need to evaluate the Boolean return value; just accept the default value in the out parameter as your 100% predictable result.

    Now the rest of your point is OK to good. Some non-Framework YourCrazyType.TryParse() could choose to stuff anything or nothing into the out parameter on failure. So a wise dev should know the documented behavior of YourCrazyType, or know the failure result is undocumented. And then code accordingly.

  • richarson (unregistered) in reply to Steve_The_Cynic

    "Obviously if they were important, they would be errors ;)"

    "-Werror is your friend... Of course, it's one of those stern and unyielding friends that everyone hates, like const in C and C++, but it's also the kind of friend that will stick by you and continue helping you no matter how much you hate it."

    That is so poetic, I'm gonna have to steal it :)

  • Gnasher729 (unregistered)

    Swift is nice - anything can be optional and nil. So you convert a string to int32 and check if the result is nil. You wouldn’t then go and replace it with zero. A text box with “0” and one with “” or “nonsense” are different.

Leave a comment on “Zero Failures”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article