Is there any way to improve this simple addition to the textbox control: appending a new line?
class CustomTextBox : TextBox
{
public void AppendLine(string textData)
{
if (this.Text.EndsWith(Environment.NewLine) || this.Text == "")
{
this.AppendText(textData);
}
else
{
this.AppendText(Environment.NewLine + textData);
}
}
}
-
\$\begingroup\$ Why are you creating a custom class, instead of writing this as an extension method? \$\endgroup\$svick– svick2014年09月11日 18:25:22 +00:00Commented Sep 11, 2014 at 18:25
-
\$\begingroup\$ Extention Methods sound like something I ought to know about, please expand or point me somewhere, thanks. \$\endgroup\$chazjn– chazjn2014年09月11日 21:48:45 +00:00Commented Sep 11, 2014 at 21:48
-
\$\begingroup\$ You could start with the official documentation: "Extension methods enable you to "add" methods to existing types without creating a new derived type, recompiling, or otherwise modifying the original type." \$\endgroup\$svick– svick2014年09月11日 22:46:02 +00:00Commented Sep 11, 2014 at 22:46
2 Answers 2
When I am thinking about changing / enhancing some code, I have usually two things (regarding the code for review) in mind:
- Whenever there is some method that gets called in every if-else / switch-case, think about using some variable to make it single call. (This includes same method on different objects of common type - not aplicable to this case, but for the record)
- Check all conditions (and variables), if they are really required. (This includes joined conditions, when one renders to be redundant because of another already met. Sometimes I see variables that are used to avoid
goto
, but it turns out it can be done without both by reorganizing it - again not aplicable to this code review, but for the check-list)
My final solution would look like this:
class CustomTextBox : TextBox {
public void AppendLine(string textToAppend) {
string current = this.Text;
if (current.Length > 0
&& !current.EndsWith(Environment.NewLine))
textToAppend = Environment.NewLine + textToAppend;
this.AppendText(textToAppend);
}
}
EDIT: I have deleted all redundant / wrong parts. The check for empty text is needed, and TextLength
is a property of WinForms TextBox, not the one in WPF.
-
\$\begingroup\$ Thanks for your answer. I need to check for empty text, otherwise the first line will only be a new line and the text will start on line 2. \$\endgroup\$chazjn– chazjn2014年09月05日 09:55:15 +00:00Commented Sep 5, 2014 at 9:55
-
\$\begingroup\$ Also we have to check the text length using
this.Text.Length
- I cannot see athis.TextLength
? \$\endgroup\$chazjn– chazjn2014年09月05日 10:06:11 +00:00Commented Sep 5, 2014 at 10:06 -
\$\begingroup\$ You are right, it must be there. And TextLength is on WinForms TextBox, probably not on WPF version. \$\endgroup\$user52292– user522922014年09月05日 11:06:56 +00:00Commented Sep 5, 2014 at 11:06
I don't care for empty quotes designating an empty string. It should be replaced with String.Empty
. It simply clarifies your intention and leaves no ambiguity that you perhaps forgot to put something in the quotes, or maybe even meant to put a space in there. Otherwise, I feel this is very simple and solid.
-
2\$\begingroup\$ I just checked and Java does not have
String.EMPTY
. I'm a bit sad. ( I know it's aC#
question, I just verified forJava
) \$\endgroup\$Marc-Andre– Marc-Andre2014年09月03日 16:17:03 +00:00Commented Sep 3, 2014 at 16:17