current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

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);
        }
    }  
}
share|improve this question
    
Why are you creating a custom class, instead of writing this as an extension method? –  svick Sep 11 '14 at 18:25
    
Extention Methods sound like something I ought to know about, please expand or point me somewhere, thanks. –  chazjn Sep 11 '14 at 21:48
    
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.” –  svick Sep 11 '14 at 22:46

2 Answers 2

up vote 2 down vote accepted

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.

share|improve this answer
    
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. –  chazjn Sep 5 '14 at 9:55
    
Also we have to check the text length using this.Text.Length - I cannot see a this.TextLength ? –  chazjn Sep 5 '14 at 10:06
    
You are right, it must be there. And TextLength is on WinForms TextBox, probably not on WPF version. –  firda Sep 5 '14 at 11:06
up vote 2 down vote

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.

share|improve this answer
2  
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 ) –  Marc-Andre Sep 3 '14 at 16:17
    
Nice tip, thanks. –  chazjn Sep 5 '14 at 9:55

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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