Try Fail...

VallarasuS

Guess Microsoft failed to mention in XML documentation TryPare != CanParse...

        string\[\] rgb = values.Split(',');
        if (rgb.Length == 3)
        {
            byte r, g, b; // <- Why is this here anyway?!
            if (byte.TryParse(rgb\[0\], out r) && byte.TryParse(rgb\[0\], out g) && byte.TryParse(rgb\[0\], out b))
            {
                color = Color.FromRgb(byte.Parse(rgb\[0\]), byte.Parse(rgb\[1\]), byte.Parse(rgb\[2\]));
                return true;
            }

Oups! wait! what are you doing?! Why are those type converters exists then?

Regards Vallarasu S | FSharpMe.blogspot.com

Lost User

r, g and b are declared because you can't call TryParse() without providing an out parameter for the result. Still, a CanParse() method would not be very much faster or more efficient than misusing TryParse() for that purpose. Anyway, I have seen much code where some rookie tried to put everything into strings (instead of proper types) and then was trying to parse (no pun intended) all over the code.

At least artificial intelligence already is superior to natural stupidity

Julien Villers

Is it really trying to parse three times rgb[0]? Then, it's an even greater WTF...

'As programmers go, I'm fairly social. Which still means I'm a borderline sociopath by normal standards.' Jeff Atwood 'I'm French! Why do you think I've got this outrrrrageous accent?' Monty Python and the Holy Grail

BobJanova

You're missing the point, I think: after calling TryParse, you already have the values in the r, g and b variables, so TRWTF is why the author then calls Parse again inside the if.

Lost User

True, but that has also become a common mistake. Many people are not used to having to use the CPU sparingly. They (edit: the CPUs, not the people :)) are now powerful enough to forgive a certain amount of wastefulness. Depending on how often this code here is executed, an optimization may make little or no observable difference. In most cases the learning effect only sets in when their practices have come back and bitten them in the rear side.

At least artificial intelligence already is superior to natural stupidity

VallarasuS

Good Spot Julien, To be honest, All I felt wrong about this piece of code is #1. This can be done with ColorConverter.ConvertFromString(string)or With TypeDescriptor.GetConverter... #2. The author invokes try parse twice, first in the predicate then within the if block I didn't notice that until now. I never thought of making it to the author but now it has to be... Thanks.

Regards Vallarasu S | FSharpMe.blogspot.com

Julien Villers

Of course you are right, if the String is a standard format, ie from serialization or sanitized user input, you should use the standard parser. If not, a TryParse done right would be the way to go.

'As programmers go, I'm fairly social. Which still means I'm a borderline sociopath by normal standards.' Jeff Atwood 'I'm French! Why do you think I've got this outrrrrageous accent?' Monty Python and the Holy Grail

Richard Deeming

VallarasuS wrote:

This can be done with ColorConverter.ConvertFromString

Or it could, if that class didn't have a nasty bug in it:

var converter = new ColorConverter();
Color color = converter.ConvertFromString("#XXXa");
Console.WriteLine(color);

Given this code, you might expect a System.FormatException or a System.ArgumentException. Instead, you get a System.Exception wrapping a System.FormatException! :wtf: So your code now becomes:

Apparently, this can't be fixed because "it may result in a breaking change in deployed applications"! https://connect.microsoft.com/VisualStudio/feedback/details/94064/colortranslator-fromhtml-throws-a-system-exception[^]

"These people looked deep within my soul and assigned me a number based on the order in which I joined." - Homer

KP Lee

That's cute, set up values, don't use them and then blow up when the if test passes and rgb[1] == "G" :laugh: Well, if it does return true, you know you "CanParse" rgb[0] :) There are a lot of things I see in code that don't make sense. Interesting, "int i = 350; byte b = (byte)i;" works fine (sorta), but if you byte.TryParse(i.ToString(), out b); it will return false while b changes from 94 to 0.