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);
        }
    }  
}
\$\endgroup\$
3
  • \$\begingroup\$ Why are you creating a custom class, instead of writing this as an extension method? \$\endgroup\$
    – svick
    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
    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
    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.

\$\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\$
    – chazjn
    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\$
    – chazjn
    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
    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.

\$\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\$
    – Marc-Andre
    Sep 3, 2014 at 16:17

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.