2
\$\begingroup\$

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);
 }
 } 
}
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Sep 3, 2014 at 15:23
\$\endgroup\$
3
  • \$\begingroup\$ Why are you creating a custom class, instead of writing this as an extension method? \$\endgroup\$ Commented 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\$ Commented 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\$ Commented Sep 11, 2014 at 22:46

2 Answers 2

2
\$\begingroup\$

When I am thinking about changing / enhancing some code, I have usually two things (regarding the code for review) in mind:

  1. 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)
  2. 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.

answered Sep 3, 2014 at 16:46
\$\endgroup\$
3
  • \$\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\$ Commented Sep 5, 2014 at 9:55
  • \$\begingroup\$ Also we have to check the text length using this.Text.Length - I cannot see a this.TextLength ? \$\endgroup\$ Commented 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\$ Commented Sep 5, 2014 at 11:06
2
\$\begingroup\$

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.

answered Sep 3, 2014 at 16:09
\$\endgroup\$
1
  • 2
    \$\begingroup\$ I just checked and Java does not have String.EMPTY. I'm a bit sad. ( I know it's a C# question, I just verified for Java ) \$\endgroup\$ Commented Sep 3, 2014 at 16:17

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.