|
I came across many of those in our code years ago from programmers that have since moved on.
The first time I saw it I was thinking "Hmmm. This probably used to have some parameters in it and got modified for some reason". After seeing 5-10 more like that I was just thinking "Hmmm." 
Soren Madsen
|
|
|
|
|
Well, I know the programmer who did this in our software.
When asked why he did that he said "I thought that would be useful in case the string ever needed parameters".
He was basically making our software 'future proof' 
It's an OO world.
public class Naerling : Lazy<Person>{
public void DoWork(){ throw new NotImplementedException(); }
}
|
|
|
|
|
Did you slap him or punch him in the stomach?
Soren Madsen
|
|
|
|
|
Nope, that would have been very unwise. That guy is my boss!
It's an OO world.
public class Naerling : Lazy<Person>{
public void DoWork(){ throw new NotImplementedException(); }
}
|
|
|
|
|
|
Right. Because adding
String.Format later would be just too hard.
Really, you actually might have to use the arrow keys or the mouse to select in the middle of the line.
In truth, I know someone who would erase and retype the entire line to add something to the middle of it. I am fairly happy to report that he is now out of code and in management instead.
|
|
|
|
|
Would that be 80 proof to match the future whiskey needed to stomach it? 
|
|
|
|
|
I've been maintaining a C++ codebase for several years now that has this type of thing all through it:
if (someErrorCondition)
{
char *msg = "Error 404\n";
char s[100];
sprintf(s, "%s", msg);
PrintErrorMessage(s);
}
as opposed to just:
if (someErrorCondition)
PrintErrorMessage("Error 404\n");
And this was in an embedded system, where they were constantly having to eliminate features because they had exceeded the limited code space or overflowed the stack!
|
|
|
|
|
I know the guy who wrote those... Gotta be the same guy....
|
|
|
|
|
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)*
|
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
