up vote 3 down vote favorite

I wanted to enable/disable an Ok button on my form if some text boxes are not empty, so I tried to use Observer pattern in the following way but something sneaky is wrong with the way I am using it so Ok button always stays disabled. What can you find wrong in this code? and what are the areas for improvement?

So here is the class:

public class InputValidator
{
    public event Action ValidationDone;
    private List<TextBox> boxes = new List<TextBox>();
    public void RegisterTextBox(TextBox tb)
    {
        tb.TextChanged += (s, e) => Validate();
        boxes.Add(tb);
    }
    public void Validate()
    {
        foreach (var t in boxes)
        {
            if (string.IsNullOrEmpty(t.Text)) return ;

        }
    }
}

and then here is how I am using it in my form:

private InputValidator validator;

public MyWinForm()
{
    InitializeComponent();
    validator = new InputValidator();
    validator.RegisterTextBox(nameTextBox);
    validator.ValidationDone += () => { OkButton.Enabled = true; };
}
share|improve this question
100% accept rate
feedback

closed as off topic by Jeff Vanzella, Brian Reichle, Corbin, Paul, James Khoury Sep 21 at 2:54

Questions on Code Review - Stack Exchange are expected to relate to code review request within the scope defined in the FAQ. Consider editing the question or leaving comments for improvement if you believe the question can be reworded to fit within the scope. Read more about closed questions here.

1 Answer

up vote 2 down vote accepted

Simplify by removing the generic list of textboxes:

public class InputValidator
{
    public event Action ValidationDone;

    public void RegisterTextBox(TextBox tb)
    {
        tb.TextChanged += (s, e) => this.Validate(s);
    }

    private void Validate(object sender)
    {
        var t = sender as TextBox;

        if ((t == null) || string.IsNullOrEmpty(t.Text))
        {
            return;
        }

        var validationDone = this.ValidationDone;

        if (validationDone != null)
        {
            validationDone();
        }
    }
}

The problem was that you were never calling the ValidationDone Action handler when the text is not empty. The above fixes that. However, you may want to handle the case for when the text is empty and you want to disable the button once again:

public class InputValidator
{
    public delegate void ValidationDoneDelegate(bool enable);

    public event ValidationDoneDelegate ValidationDone;

    public void RegisterTextBox(TextBox tb)
    {
        tb.TextChanged += (s, e) => this.Validate(s);
    }

    private void Validate(object sender)
    {
        var t = sender as TextBox;

        if (t == null)
        {
            return;
        }

        var validationDone = this.ValidationDone;

        if (validationDone != null)
        {
            validationDone(!string.IsNullOrEmpty(t.Text));
        }
    }
}

and call it like this:

    private InputValidator validator;

    public MyWinForm()
    {
        InitializeComponent();
        validator = new InputValidator();
        validator.RegisterTextBox(nameTextBox);
        validator.ValidationDone += (enable) => { OkButton.Enabled = enable; };
    }
share|improve this answer
feedback

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