|
Was the code ported from C++?
I haven't seen this one before but definitely seen stuff like this when using a tool to convert from one language to another.
"You get that on the big jobs."
|
|
|
|
|
This was newly written code under an existing ASP.NET C# code base. It was written by a "supposedly" senior developer. 
No matter where you go, there you are...~?~
|
|
|
|
|
Tell the original developer to use this
StringBuilder sb = new StringBuilder();
sb.Append(string.empty);
sb.Append(funcThatReturnsAString());
sb.Append(string.empty);
value = sb.ToString();
Tell him this is the standard way of doing this kind of stuff 
Every now and then say, "What the Elephant." "What the Elephant" gives you freedom. Freedom brings opportunity. Opportunity makes your future.
|
|
|
|
|
Guess Microsoft failed to mention in XML documentation TryPare != CanParse...
string[] rgb = values.Split(',');
if (rgb.Length == 3)
{
byte r, g, b;
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?
|
|
|
|
|
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
|
|
|
|
|
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.
|
|
|
|
|
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
|
|
|
|
|
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
|
|
|
|
|
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.
|
|
|
|
|
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
|
|
|
|
|
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! 
So your code now becomes:
var converter = new ColorConverter();
try
{
Color color = converter.ConvertFromString("#XXXa");
Console.WriteLine(color);
}
catch (FormatException)
{
}
catch (Exception ex)
{
if (ex.InnerException == null) throw;
if (!(ex.InnerException is FormatException)) throw;
}
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
|
|
|
|
|
That's cute, set up values, don't use them and then blow up when the if test passes and rgb[1] == "G" 
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.
|
|
|
|
|
value = String.Concat(value, intValue.ToString(), ".");
logBuilder = String.Concat("some long text",
"some long text. ");
String errorMessage = String.Concat("Long text, exception: ", ex.ToString());
Find all "String.Concat", Subfolders, Find Results 1, "Current Project"
...
Matching lines: 297 Matching files: 32 Total files searched: 106
|
|
|
|
|
And this is the best:
String errorMessage = String.Concat(".... packet is fail, exception: ", ex.ToString());
SomeClass.someMember.Error(String.Concat(errorMessage));
|
|
|
|
|
String.Concat("Someone has fallen in love with", "String.Concat");
(yes|no|maybe)*
|
|
|
|
|
|
That has to be him!
public class SysAdmin : Employee
{
public override void DoWork(IWorkItem workItem)
{
if (workItem.User.Type == UserType.NoLearn){
throw new NoIWillNotFixYourComputerException(new Luser(workItem.User));
}else{
base.DoWork(workItem);
}
}
}
|
|
|
|
|
This is just as bad as my example of String.Format.  
No matter where you go, there you are...~?~
|
|
|
|
|
If you've read anything about strings, you must have read that you should use StringBuilder instead. "some text" + "some more" + " text" is expensive. I would guess Concat gives you a performance boost similar to StringBuilder, but limited to strings. I have to admit that appending each value individually in builder is bothersome.
I know I wanted to find out how much builder helps, so I built a cpu intensive process with accounting numbers. I mixed fixed text with numbers to a string using + logic. Got the actual coding to work and changed over to StringBuilder. Processing went from 40 minutes to 20 minutes. Then I changed the logic to only use builder when I needed to save the stat strings. Cut down to 7 minutes. Then played with the process order and it went to 4 minutes. Needed to buy my second laptop in 7 years to cut it to 2 minutes.
|
|
|
|
|
Would someone do this:
public IEnumerable<TrackingTagSpan<T>> GetTaggedSpans(SnapshotSpan span)
{
IList<TrackingTagSpan<T>> source = new List<TrackingTagSpan<T>>(this._trackingTagSpans);
lock (this.mutex)
{
source = new List<TrackingTagSpan<T>>(this._trackingTagSpans);
}
return
from tagSpan in source
where span.IntersectsWith(tagSpan.Span.GetSpan(span.Snapshot))
select tagSpan;
}
This is from a builtin class (SimpleTagger<T>) in the Visual Studio SDK.
Edit: Looks like I upset a Microsoftie. Instead of downvoting, go fix the f#@kin crap code buddy.
modified 14-Jun-12 7:56am.
|
|
|
|
|
This guy had his head full of coffee (or other drugs, can't say exactly)
Maybe the code where longer before and was shortened to this
I'm brazilian and english (well, human languages in general) aren't my best skill, so, sorry by my english. (if you want we can speak in C# or VB.Net =p)
|
|
|
|
|
Would a simple error be a ground-breaking concept for you?
|
|
|
|
|
Vitaly Tomilov wrote: Would a simple error be a ground-breaking concept for you?
What is that suppose to mean?
|
|
|
|
|
Looks like a simple error, which needs a simple fix.
|
|
|
|
|
Vitaly Tomilov wrote: Looks like a simple error, which needs a simple fix.
Firstly, simple error yes, but why is this not spotted a mile away?
Secondly, this is not my code. It is Microsoft's production code, that lives in VS2010 and VS2012. It is up to them to fix such a simple error.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
