TAGS :Viewed: 7 - Published at: a few seconds ago

[ get and Set not working ]

I am writing the following get and set for validating an input from a Text Box. Basically it is supposed to check if the user has entered all of the values. When I leave the TextBoxes empty , it does nothing and shows a '0' in output where that variable was being used. It does however show the system generated exception and stops the execution, but I wonder why doesn't it validate the input through the properties?

Here is my code:

public double RecoDoseSize
{
    get
    {
        return recoDoseSize;
    }
    set
    {
        if (!(value>0))
        {
            MessageBox.Show("Please Enter the recommended dose size for this product");
            textBox8.Focus();
        }
        recoDoseSize = value;
    }
}

private void Submit2_Click(object sender, RoutedEventArgs e)
{
    TotalContentProduct = double.Parse(textBox7.Text);
    recoDoseSize = double.Parse(textBox8.Text);
    NoOfDosespUnit = TotalContentProduct/recoDoseSize;
}

Answer 1


You are setting recoDoseSize, the backing field, not RecoDoseSize, the property which has your code in it. Thus, your code isn't executed. You need to change the second line of your method body to

RecoDoseSize = double.Parse(textBox8.Text);

(note the capital R).

Answer 2


You're never actually using the getter/setter. You are using the actual field name: recoDoseSize directly. Change it to RecoDoseSize.

Answer 3


private void Submit2_Click(object sender, RoutedEventArgs e)
{
    TotalContentProduct = double.Parse(textBox7.Text);
    RecoDoseSize= double.Parse(textBox8.Text);
    NoOfDosespUnit = TotalContentProduct/recoDoseSize;
}

Answer 4


Other have given the correct answer to the question as stated. Namely that you should call the uppercased RecoDoseSize if you want to use the getter/setter.

However it is extremely bad practice to show a message box inside the setter, because it violates the Principle of Least Surprise.

When someone looks at the line RecoDoseSize = double.Parse(textBox8.Text); it is not at all obvious that this operation could cause a message box to appear.

There are occasionally exceptions where it does make sense to have a setter trigger UI changes (for instance the Visible property on controls) however the default should always be to not do this unless you are sure it will be more confusing to not do so (for instance it would be surprising if you set Visible = false however it was still visible).

Regarding your comment on how you should implement it, the checking should be done in the click handler and the property can just be an auto-property, like so:

public double RecoDoseSize { get; set; }

private void Submit2_Click(object sender, RoutedEventArgs e)
{
    TotalContentProduct = double.Parse(textBox7.Text);

    double enteredSize;
    if (!double.TryParse(textBox8.Text, out enteredSize) || enteredSize <= 0)
    {
        MessageBox.Show("Please Enter the recommended dose size for this product");
        textBox8.Focus();
        return;
    }
    RecoDoseSize = enteredSize;
    NoOfDosespUnit = TotalContentProduct / recoDoseSize;
}

You'll want to use TryParse because with Parse you'll get an error if the text isn't a valid double. What TryParse does is return true or false depending on whether it succeeded, and it populates the out parameter with the result if it's successful.

So what this does is if it either failed to parse the result, or the result is <= 0 it shows the message box. In that case it also returns from the method so the rest of it isn't executed. Alternatively the rest of the method could be in an else block in which case the return isn't needed. It's a matter a style which way is preferred.

Answer 5


You shouldn't be handling focus in your set statement.

Also, you need to make sure that value is not null, otherwise you can't compare it to anything (greater-than, etc.).