current community

your communities

more stack exchange communities

Stack Exchange Inbox Reputation and Badges
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

I have been working on Custom TextBox control, which only allows alphanumeric characters. Note that I have not restricted characters from KeyPress, KeyUp event as I don't want to restrict copy/paste or any other operation that should be allowed generally. I have only trimmed non-alphanumeric character on paste operation. However, I am not sure whether the code I wrote is good or bad as I have very little experience of desktop application.

using System;
using System.Windows.Forms;
using System.Text.RegularExpressions;

namespace SaintThomas.UserControls
{
    class TextBoxForUserName : TextBox
    {
    protected override void OnTextChanged(EventArgs e)
    {
        base.OnTextChanged(e);

        this.SuspendLayout();
        int startPos = this.SelectionStart;
        if (Regex.IsMatch(this.Text, "[^0-9_A-Z]", RegexOptions.IgnoreCase))
        {
        int reduceStartPos = this.Text.Length;
        this.Text = Regex.Replace(this.Text, "[^0-9_A-Z]", "", RegexOptions.IgnoreCase);
        startPos = (startPos <= 0) ? 0 : startPos - (reduceStartPos - this.Text.Length);
        if (this.Text.Length < startPos)
        {
            startPos = this.Text.Length;
        }
        this.SelectionStart = startPos;
        }
        this.ResumeLayout();
    }
    }
}
share|improve this question

2 Answers 2

up vote 1 down vote

Are you reinventing the MaskedTextBox?

share|improve this answer
    
Of course not! If the functionality I tried to implement through above code is possible using MaskedTextBox then please let me. Also note that the length of text can be vary and I don't want PromtChar appear in textbox. –  Parth Patel Dec 21 '11 at 12:51
    
It seems to be, but I guess one needs to try it (I haven't). I knew that these masked controls existed so figured it would be useful to post it. –  ANeves Dec 21 '11 at 16:53
up vote 1 down vote

Aside from the indentation issues...

  • You could simply use @"\W" for your regular expression (with RegexOptions.None).
  • A simpler way to deal with the cursor is to measure the distance from the end of the string, since you can know that will remain constant.
  • Using Regex.Match followed by Regex.Replace seems pointless to me. If the string doesn't need to be changed, Regex.Replace should be able to determine that quickly.

  • Calls to SuspendLayout also seem unnecessary, since the layout of your control (i.e., its bounds, position) is not changing.

    protected override void OnTextChanged(EventArgs e) {
    base.OnTextChanged(e);
    int charsFromEnd = this.Text.Length - this.SelectionStart;
    this.Text = Regex.Replace(this.Text, @"\W", "", RegexOptions.None);
    this.SelectionStart = this.Text.Length - charsFromEnd;
}
share|improve this answer

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.