Printable View
If you always know that you want to have no spaces, you can call trim as soon as you save off the value:
You don't need to check for null here as you know the Text property of a textbox will never return null.Code:string lastName = textbox.Text.Trim();
if (lastName != String.Empty)
{
...
}
In this case it is ok to do:Quote:
Originally Posted by dee-u
string s = textBox.Text.Trim();
since the text box cannot have a null text.Code:if(String.IsNullOrEmpty(s)
{
//use the value
}
Though I would argue that it is better to just take away the spaces to begin with if you know they should not be there. This way you won't run into trouble down the road using that variable. At least this way you would not need to call Trim() twice.
determining whether the textbox is empty is one thing, validating input is another. Personally, I like to use an IExtenderProvider textbox validator that utilizes a regex to validate the input and can fire an event for invalid data. that way, validation can be added on a per-texbox basis in design mode, and all I have to do is handle a single event on the input validator extender for validation. combined w/ an error extender, its very simple.
its just much simplier than adding tons of code to check for null (since a textbox's text property will never be null), length, and content.
100% agreement. Input validation should be done with Validators whenever possible, and should NOT be part of the "normal" processing code. The latter makes it too easy to miss something, obsures the real purpose of the code, and introduces many more opportunities for inconsistancy.Quote:
Originally Posted by MadHatter
I'm sure you've all heard of the database tenet of garbage in/garbage out or, in other words, only store valid data into the database.
If we apply this concept to variables, then you'll only want to store valid data inside variables.
If you are getting external data (like from parsing a file for example), trim/validate the data before storing it.
If you get data from a textbox do the same before storing it (like using validators as suggested).
If you follow this style of programming, once a valid string is entered into a variable, then only the most basic check needs to be performed using String.IsNullOrEmpty(...).
As a final comment, I would still use IsNullOrEmpty(...) after validating text from a textbox even though the textbox text can't ever be null. Why? because it makes the code simpler and more consistent.
No matter how you code to check if a given string is a null string or not, the compare function is always called. Since the text length is 0 the execution takes O(1) time-complexity. Many not-so-oftenly-used functions to check for null string might include several other code which may be optimized away by the compiler, which might not be advised to use for internal structure branching conditions might be involved.
That may be true nor C++ type strings (e.g. null terminated, std::string, etc) but is NOT ture for .NET's System.String - just look at the source code!Quote:
Originally Posted by Khiem
Disregarding null values of a string I'd think using the Length is still the fastest so I'd say it is best. If anyone could improve the benchmark that I did then pls. clarify.
With Trim:
Without Trim:Code:private void button1_Click(object sender, EventArgs e)
{
Stopwatch stopWatch = new Stopwatch();
string s = " ";
string result = string.Empty;
stopWatch.Reset();
stopWatch.Start();
for (int a = 1; a <= 10000; a++)
{
s = " ";
Console.WriteLine(s.Trim().Length == 0);
}
stopWatch.Stop();
result =stopWatch.ElapsedMilliseconds.ToString() + " = Using Length";
stopWatch.Reset();
stopWatch.Start();
for (int a = 1; a <= 10000; a++)
{
s = " ";
Console.WriteLine(String.IsNullOrEmpty(s.Trim()));
}
stopWatch.Stop();
result += Environment.NewLine + stopWatch.ElapsedMilliseconds.ToString() + " = Using IsNullOrEmpty";
stopWatch.Reset();
stopWatch.Start();
for (int a = 1; a <= 10000; a++)
{
s = " ";
Console.WriteLine(s.Trim() == string.Empty);
}
stopWatch.Stop();
result += Environment.NewLine + stopWatch.ElapsedMilliseconds.ToString() + " = Using string.Empty";
MessageBox.Show(result);
stopWatch = null;
}
Code:private void button2_Click(object sender, EventArgs e)
{
Stopwatch stopWatch = new Stopwatch();
string s = string.Empty;
string result = string.Empty;
stopWatch.Reset();
stopWatch.Start();
for (int a = 1; a <= 10000; a++)
{
Console.WriteLine(s.Length == 0);
}
stopWatch.Stop();
result = stopWatch.ElapsedMilliseconds.ToString() + " = Using Length";
stopWatch.Reset();
stopWatch.Start();
for (int a = 1; a <= 10000; a++)
{
Console.WriteLine(String.IsNullOrEmpty(s));
}
stopWatch.Stop();
result += Environment.NewLine + stopWatch.ElapsedMilliseconds.ToString() + " = Using IsNullOrEmpty";
stopWatch.Reset();
stopWatch.Start();
for (int a = 1; a <= 10000; a++)
{
Console.WriteLine(s == string.Empty);
}
stopWatch.Stop();
result += Environment.NewLine + stopWatch.ElapsedMilliseconds.ToString() + " = Using string.Empty";
MessageBox.Show(result);
stopWatch = null;
}
well first, you're using an unnecessary increment operation in your for loops (lest I digress... prefix increment is what you should be using).
second, the implementation of string.IsNullOrEmpty contains the following code:
so the value != null instruction is pointless here since TextBox.Text is never going to be null, and it ends up using the string.Length property, so whether you use the text's length property, or string.IsNullOrEmpty, you're going to be accessing the string's Length property, so skipping one instruction will save you a tick.Code:public static bool IsNullOrEmpty(string value) {
if (value != null) {
return (value.Length == 0);
}
return true;
}
having said that, looking at the implementation of TextBox.Text to see that its value will never be null could be considered a premature optimization by the purest because the Text property is never documented to always contain a value, so on that note, I'd probably opt for a string.IsNullOrEmpty, though I've seen lots of code written prior to .NET 2.0 that writes out the text != null && text.Length > 0 since that method wasn't available at the time the code was written. There is a lot to be said about consistency.
I don't think it really matters if it is faster or not. how often do you need to eek out performance from a check for an empty string? I go with readability first, performance...10th, once I am sure it is necessary. That's not to say that I knowingly write inefficient code, just that no one will notice a lost millisecond on a string compare, even if your benchmark is correct, which I doubt for String.Empty as it is a static constant.Quote:
Originally Posted by dee-u
Isn't .Length not readable enough?
well, it's not that bad, but I just prefer String.Empty or IsNullOrEmpty() as it is about as clear as possible. I'm not saying go and rewrite your code.
We should agree that it boils down to the personal inclination of a developer or organization, some would prefer performance over readability and readable for some can be not that readable to others, do you agree?
I find it hard to believe that:
is appreciably faster than...Code:if (str.Length == 0)
...in almost every single situation. It is about three - four times as fast as far as I can tell, but unless you have a string compare in your video game's inner loop it probably will be meaningless.Code:if (str == String.Empty)
...and the source for IsNullOrEmpty:
Exactly what you would have to do to protect against the crash pointed out by CPUWizard.Code:public static bool IsNullOrEmpty(string value)
{
if (value != null)
{
return (value.Length == 0);
}
return true;
}
I can't agree that this type of optimization should win out over readability in 99% of the cases, but yeah, it's not a big deal and is not going to cause issues. Any programmer who doesn't get what str.Length == "" means should be fired anyway.