For starters, don't use Convert methods with user input - if the value they enter doesn't match the right format, your application will crash. Use the TryParse methods instead:
double height, weight;
double BMI = 0;
if (!double.TryParse(textBox1.Text, out height))
{
MessageBox.Show($"\"{textBox1.Text}\" is not a valid height");
return;
}
if (!double.TryParse(textBox2.Text, out weight))
{
MessageBox.Show($"\"{textBox3.Text}\" is not a valid weight");
return;
}
Secondly, you are missing cases where values are equal: 18.5, between 24.9 and 25, between 29.9 and just under 30.
Change your tests to these, and you cover all cases (as well as making your code more readable):
if (BMI < 18.5)
{
...
}
else if (BMI < 25.0)
{
...
}
else if (BMI < 30.0)
{
...
}
Thirdly, especially when you are getting started, always use curly brackets even if they aren't needed, that way your code is more obvious:
else if (BMI >= 30)
label3.Text = Convert.ToString(" obese");
label3.forecolor="Color.Red;
Is the equivalent of:
else if (BMI >= 30)
{
label3.Text = Convert.ToString(" obese");
}
label3.forecolor="Color.Red;
That's not the same as this:
else if (BMI >=30)
{
label3.Text = Convert.ToString("Obese");
label3.ForeColor = Color.Red;
}
And it means that the ForColor is set to Red every time!
You also have no method for resetting a colour after someone is overweight - so when you set the BackColor to Green, it never returns to white again.
A better way to do this is like this:
BMI = weight / (height * height);
string text = "Major Error!";
Color backColor = SystemColors.Window;
Color foreColor = SystemColors.WindowText;
if (BMI < 18.5)
{
text = "Under Weight";
foreColor = Color.Blue;
}
else if (BMI < 25.0)
{
text = "Normal";
foreColor = Color.Black;
}
else if (BMI < 30.0)
{
text = "Over Weight";
backColor = System.Drawing.Color.Green;
foreColor = Color.White;
}
else
{
text = Convert.ToString("Obese");
foreColor = Color.Red;
}
label3.Text = text;
label3.ForeColor = foreColor;
label3.BackColor = backColor;
Note that I got rid of your spurious
Convert.ToString
calls as well - you don't need to convert a string to a string, that's just silly, and makes your code harder to read.
Finally, do yourself a favour, and stop using Visual Studio default names for everything - you may remember that "TextBox8" is the mobile number today, but when you have to modify it in three weeks time, will you then? Use descriptive names - "tbMobileNo" for example - and your code becomes easier to read, more self documenting, easier to maintain - and surprisingly quicker to code because Intellisense can get to to "tbMobile" in three keystrokes, where "TextBox8" takes thinking about and 8 keystrokes...