Question BMI Calculator

[Paradoxz1]

Hi,

I have been trying for the past 6 days to get this BMI calculator working to my requirements but to no success. I am not even able to get a value out from it. I don't know where to start looking for the issue as I have been looking at this code for hours now and been looking at different guides and such but I am no closer to getting this working. This forum is my last resort as it has been with previous projects.

As I don't know where to issue lies ill post my code and some images in hopes that one of you can help me figure out where the issue lies. I don't want you to send me a working code as response but help me figure out where the issue is and what method I should use in order to solve it.

Form1.cs [Design]

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace Assignment_3_New
{
    public partial class Form1 : Form
    {
        private BMICalculator bmiCalc = new BMICalculator();
        public Form1()
        {
            InitializeComponent();
            InitializeGUI();

        }
        private void InitializeGUI()
        {
            this.Text = "The Body Mass Calculator by [Name]";

            //input
            radbtnMetric.Checked = true;
            lblHeight.Text = "Height (cm)";
            lblWeight.Text = "Weight (kg)";

            //output
            txtHeight.Text = string.Empty;
            txtWeight.Text = string.Empty;
        }

        private void DisplayResults()
        {
            lblBMICalculated.Text = bmiCalc.CalculateBMI().ToString("f2");
            lblWeightCatCalculated.Text = bmiCalc.BMIWeightCategory().ToString();
            grpResult.Text = "Results for " + bmiCalc.GetName();
        }

        private void label1_Click(object sender, EventArgs e)
        {

        }
        //********************************************* Read Weight *********************************************
        #region Read Weight
        private bool ReadWeight()
        {
            double outValue = 0;
            bool ok = double.TryParse(txtWeight.Text, out outValue);

            if (ok)
            {
                if (outValue > 0)
                {
                    bmiCalc.SetWeight(outValue);
                }
                else
                    ok = false;
            }
            if (!ok)           
                MessageBox.Show("Invalid weight value!", "Error");

            return ok;
            
        }
        #endregion
        private void textBox3_TextChanged(object sender, EventArgs e)   //Weight
        {

        }

        private void textBox1_TextChanged(object sender, EventArgs e)   //Name
        {
            bmiCalc.SetName(txtName.Text);
        }

        //********************************************* Read Height *********************************************
        #region Read Height
        private bool ReadHeight()
        {
            double outValue = 0;
            bool ok = double.TryParse(txtHeight.Text, out outValue);

            if (ok)
            {
                if (outValue > 0)
                {
                    if (bmiCalc.GetUnit() == UnitTypes.American)
                    {
                        bmiCalc.SetHeight(outValue * 12.00);
                    }
                    else
                    {
                        bmiCalc.SetHeight(outValue / 100.0);
                    }
                }
                else
                    ok = false;
            }
            if (!ok)
                MessageBox.Show("Invalid height value!", "Error");

            return ok;
        }

        #endregion
        private void textBox2_TextChanged(object sender, EventArgs e)   //Height
        {

        }

        private void textBox5_TextChanged(object sender, EventArgs e)
        {

        }

        private void textBox4_TextChanged(object sender, EventArgs e)
        {

        }

        private void lblWeightCat_Click(object sender, EventArgs e)
        {

        }

        private void lblBMI_Click(object sender, EventArgs e)
        {

        }

        private void grpResult_Enter(object sender, EventArgs e)
        {

        }
        //********************************************* Read Input BMI *********************************************
        #region Read Input BMI
        /*private bool ReadInputBMI()
        {
            bool CalculateClicked = false;

            btnCalculate.Click
        }*/
        #endregion
        private void btnCalculate_Click(object sender, EventArgs e)
        {
            /*bool ok = ReadInputBMI();

            if (ok)
            {
                DisplayResults();
            }*/
            DisplayResults();

        }

        private void grpUnit_Enter(object sender, EventArgs e)
        {

        }

        private void radbtnUs_CheckedChanged(object sender, EventArgs e)
        {
            if (radbtnUs.Checked)
            {
                lblHeight.Text = "Height (inch)";
                lblWeight.Text = "Weight (lbs)";
                bmiCalc.SetUnit(UnitTypes.American);
            }
        }

        private void radbtnMetric_CheckedChanged(object sender, EventArgs e)
        {
            if (radbtnMetric.Checked)
            {
                lblHeight.Text = "Height (cm)";
                lblWeight.Text = "Weight (kg)";
                bmiCalc.SetUnit(UnitTypes.Metric);
            }
        }

        private void WeightCat_Click(object sender, EventArgs e)
        {

        }

        private void lblBMICalculated_Click(object sender, EventArgs e)
        {

        }
    }
}

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Assignment_3_New
{
    class BMICalculator
    {
        private string name = "No name";
        private double height = 0;
        private double weight = 0;
        private UnitTypes unit;

        // ********************************************* Getters *********************************************
        #region Getters
        public string GetName()
        {
            return name;
        }

        public double GetHeight()
        {
            return height;
        }

        public double GetWeight()
        {
            return weight;
        }

        public UnitTypes GetUnit()
        {
            return unit;
        }
        #endregion
        // ********************************************* Setters *********************************************
        #region Setters
        public void SetName(string value)
        {
            if (!string.IsNullOrEmpty(value))
            {
                name = value;
            }
        }

        public void SetHeight(double value)
        {
            if(value >= 0)
            {
                height = value;
            }
        }

        public void SetWeight(double value)
        {
            if(value >= 0)
            {
                weight = value;
            }
        }

        public void SetUnit(UnitTypes value)
        {
            unit = value;
        }
        #endregion
        //********************************************* Calculate BMI *********************************************
        #region Calculate BMI
        public double CalculateBMI()
        {
            double bmi = 0.00;
            if(unit == UnitTypes.American)
            {
                 bmi = 703 * weight / (height * height);
            }
            else if(unit == UnitTypes.Metric)
            {
                 bmi = weight / (height * height);
            }
            return bmi;
        }
        #endregion
        //********************************************* BMI Weight Category *********************************************
        #region BMI Weight Category
        public string BMIWeightCategory()
        {
            double bmi = CalculateBMI();
            string stringout = string.Empty;
            if (bmi >= 30)
                stringout = "Obesity";
            else if (bmi < 30)
                stringout = "Overweight";
            else if (bmi < 25 && bmi >= 18.5)
                stringout = "Normal Weight";
            else
                stringout = "Underweight";

            return stringout;
        }
        #endregion
      
    }
}

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Assignment_3_New
{
    enum UnitTypes
    {
        Metric,
        American
    }
}

I realise this is a big ask, but I felt I had exhausted all my other options as the material I have to look at for help IMO not very good and well structured and I have been trying to solve this with the help if that for the past 6 days to no avail.

Results when running program

No matter the input on Height or Weight the "Weight Category" is always 'Underweight' and "BMI" is always whatever those symbols are. Even though I have tried to use double.TryParse to make a messagebox popup when Height and Weight are invalid (non numbers) the results are the same for Metric and Imperial (Us) units.

To end this long post, id like to really thank anyone who has read trough all of this as for me atleast this is alot of code and I would be really greatful for any input on the matter, as I said this is my last resort and im not looking for handout merley help on where to even start as when I look at the code with my eyes I feel like it should work but then again im new to this and the results show that it is not working so ye...

Best regards

 

[jmcilhinney]

There is way, waaaaaaaaayyyyyyyyyy too much code there. The fact that you have dumped every bit of code you have indicates that you have not made reasonable efforts to isolate the issue. Your whole application is not a single problem. Break the whole process down into smaller parts and work on each part in isolation. At the very least, you can break it down to getting user input, performing the calculation and displaying output. Those are three completely independent operations and can be treated as such. You can write code to perform the calculation using canned data and see if that works. If it does then the calculation is not the issue and is basically irrelevant to us. If it doesn't work then the input and output parts are irrelevant and you need to work on just the calculation, breaking it down into smaller parts and debugging the code to determine exactly where and how its behaviour doesn't meet your expectations. That is what software development is and that's what you need to do here. Once you have isolated where and how the issue manifests, you can provide us with only the information relevant to that, which might even involve copying just the relevant code into a completely separate test program. This is basically problem-solving 101 (divide and conquer) and is not specific to programming, but it is essential to programming. Try that and get back to us when you have narrowed things down a bit (a lot, actually).

 

[Paradoxz1]

Edited: I'm sorry if anyone had enough time to read this before I edited it out working on a solution will update
If you did see this post before edit please disregard

 

[Skydiver]

I may need to take a closer look at the code above on a bigger screen, but just scanning through the code on my phone, it looks like you don't actually pull the values from the textboxes and pass them into your calculator. It looks like the calculator is just asked to compute directly with whatever values it currently has.

That would explain why the results never change, but I don't know why you would get the strange square boxes for the computed BMI. Calling ToSting("f2") on any number should still result in a string that has printable glyphs. Those square boxes usually indicate glyphs that are not present in the selected font for the TextBox.

 

[Paradoxz1]

I did some work on it and managed to get it to print correct values it was as you said I never did pull the values which I now have fixed but upon fixing that I now have another issue which I cant seam to fix, Since I'm using the TryParse method to only parse numbers and if not show a messagebox and whenever I input a none numeric string it pops the error box, which I want only when the "calculate" button has been pressed not every time I accidental put something other than a number in. For example when I try and erase the height I had input and I come to the last (175 and I delete 5 and 7) so when I come to "1" and press backspace the error message prompts same with any none numeric number (abcdefg...) I only want it to prompt the Error box when the calculate button has been pressed and the TryParse failed. So I created a bool to check if the button had been pressed like so:

        private bool ReadInputBMI = false;

        private void btnCalculate_Click(object sender, EventArgs e)
        {
            ReadInputBMI = true;

            DisplayResults();
            ReadInputBMI = false;


        }

And then I tried implementing that before the Try.Parse method but whenever I try and do that it breaks my code and my inputs never gets set.

         private void ReadWeight()
                {
                    if(ReadInputBMI == true)
                    {
                        double outValue = 0;
                        bool ok = double.TryParse(txtWeight.Text, out outValue);

                        if (ok)
                        {
                            if (outValue > 0)
                            {
                                bmiCalc.SetWeight(outValue);
                            }
                            else
                                ok = false;
                        }
                        if (!ok)
                            MessageBox.Show("Invalid weight value!", "Error");
                    }


                }
        private void textBox3_TextChanged(object sender, EventArgs e)   //Weight
        {
            ReadWeight();
        }

I have debuged and I can confirm that my number I input never get set but I don't see why or how I would go about, doing this another way to check if the button has been pressed and then run it once and then set the button click to false again

 

[jmcilhinney]

Your explanation is barely coherent so I'm going to ignore what you're doing and explain how I think it should be done. A BMI calculation is based on height and weight so it doesn't make sense to allow the user to even try to calculate a BMI if they haven't entered a height and a weight. In that case, the Button should be disabled unless valid data has been entered. The proper way to validate in WinForms is to handle the Validating event of a control and set e.Cancel to true if the data fails validation. That way, the control will refuse to lose focus as long as it contains invalid data. You can also handle the Validated event to execute code only when control passes validation. In this case, I would do something like this:

private void textBoxes_Validating(object sender, System.ComponentModel.CancelEventArgs e)
{
    // Get the TextBox that raised the event.
    var tb = (TextBox)sender;

    // Check whether the control contains text and that that text is a valid representation of a number.
    if (tb.TextLength > 0 && !double.TryParse(tb.Text, out _))
    {
        // Validation has failed.

        // Highlight the input.
        tb.HideSelection = false;
        tb.SelectAll();

        // Prompt the user with specific information from the TextBox being validated.
        MessageBox.Show($"Please enter a valid {tb.Tag}");

        // Remove highlight without focus.
        tb.HideSelection = true;

        // Don't allow the control to lose focus.
        e.Cancel = true;
    }
}

private void textBoxes_Validated(object sender, EventArgs e)
{
    // We know that the current TextBox is valid. Enable the button if and only if both are valid.
    // This assumes that there are no other [ICODE]TextBoxes[/ICODE] on the form. If there are, use some other method to isolate them, e.g. a [ICODE]Panel[/ICODE] or an explicit list.
    button1.Enabled = Controls.OfType<TextBox>().All(tb => tb.TextLength > 0);
}

private void button1_Click(object sender, EventArgs e)
{
    // The Button must be enabled so we know that the data is valid.
    var height = double.Parse(textBox1.Text);
    var weight = double.Parse(textBox2.Text);

    // Use data here.
}

Note that one method is used to handle an event for both TextBoxes because the code is the same in each case. For the prompt displayed when validation fails, the name of the value being entered in stored in the control itself and retrieved from there. Each TextBox will refuse to lose focus if it contains invalid data. They will lose focus if they are blank but the Button will not be enabled in that case. The Button is enabled if and only if both TextBoxes contain valid data so, when it is clicked, you can assume that the data is valid. Note that validation only occurs when the user tries to navigate away from a TextBox, so the user can enter and delete whatever text they like before that. Note that the Button should be disabled by default, because both TextBoxes do not contain valida data by default.

I should also point out what's going on here:

if (tb.TextLength > 0 && !double.TryParse(tb.Text, out _))

The underscore character there is what's known as a discard. It's basically a variable that you will never use. You could do this:

if (tb.TextLength > 0 && !double.TryParse(tb.Text, out var value))

or this:

double value;

if (tb.TextLength > 0 && !double.TryParse(tb.Text, out value))

but that makes your code more verbose for no reason. Anyone looking at that will see you set that value variable and may wonder whether you forgot to use it. By using a discard, you are implicitly stating that you specifically intend not to use the value it receives. Discards are a fairly recent addition to C# syntax.

 

[Paradoxz1]

Thank you jmcilhinney, this was all new to me ill go through the code and try to understand it so thanks for the comments and the explanations. It was very different from how I was thinking it could work. I have only done C++ coding in Arduino before. I'm sorry if my explanation was not understandable as in my own head I barely understood what I was trying to explain. But I will try and give it another shot as I think this is a bit too advanced for me, I feel it should still be doable with what I have in mind.

The current issue is that whenever I enter an invalid number (without pressing the button) I get prompted with the MessageBox, I don't want this. I want to message box to only prompt if I have entered an invalid value and the "calculate button" has been pressed. I ger prompted that there is an invalid number and I need to change it and then I can try and press calculate again and if all numbers are valid (they pass Try.Parse) then display BMI.

I understand that this might be a "worse" way of writing the code in comparison to the validation you wrote above but it just makes sense in my head I just can't get it into code. Also, I should mention that I'm taking a course on C# but with covid going on its all online with over 350 students in my course the teachers are very limited and thus I'm using this forum to help me. I have pointed out previously I'm not looking for handouts as I am actually trying to learn and understand the code but with that said there are certain requirements I need to meet and I feel going and using something like validation that we have not yet covered would not be the best course of action for me. I hope you understand and can if willing get back to me if it is possible to create code as I have described. Sorry if my writing skills complicated things it's 6 am and I'm yet to go to sleep. Zzzz

 

[jmcilhinney]

The current issue is that whenever I enter an invalid number (without pressing the button) I get prompted with the MessageBox, I don't want this. I want to message box to only prompt if I have entered an invalid value and the "calculate button" has been pressed.

That is pretty much exactly backwards and the code I provided will prompt the user when they try to leave a control that contains invalid data. As I said, it is better UX to not even allow the user to click the Button if it can't possibly do anything useful.

That said, if you really want to do it backwards, i.e. not prompt the user unless they click the Button, then why are you executing code that will prompt the user any time other than when the user clicks the Button? If the only time you want to validate the input and perform the calculation is when the user clicks the Button then the only place you should be executing code that validates the input and performs the calculation is in the Click event handler of the Button. Why are you executing any code at all on the TextChanged event of a TextBox?

This is an example of why you don't try to go from an idea in your head to code. You should be working out the logic first, before writing any code at all. If that requires you to write things down with a pen and paper then so be it. Once you think that you have sound logic, you then write code to implement that logic specifically. You always have something to compare the code to then. If some of the code doesn't work then you can always point to the specific logic that that code was supposed to implement and we can tell you why it doesn't. If you just have random code for no reason that you can explain, how are we supposed to tell you what it should look like? There has to be a specific purpose for there to be a right way for the code to look.

 

[Paradoxz1]

Umm ye thanks, that fixed it for me. I moved ReadHeight and ReadWeight from the textbox to the Button click, makes sense I'm sorry if I've made myself sound stupid in this conversation but Ill take a look at the validation code you sent me as I understand how it would be more logical and efficient to do something like that.

 

[JohnH]

Does your requirement allow you to use proper controls for input? The NumericUpDown control is like a TextBox but will only take numeric input, if you use that you can skip the manual validation and conversion of input.

 

[jmcilhinney]

Does your requirement allow you to use proper controls for input? The NumericUpDown control is like a TextBox but will only take numeric input, if you use that you can skip the manual validation and conversion of input.

One additional advantage of using a NumericUpDown is that you get to specify a range of allowed values. That means that you can easily exclude negative values, which are obviously meaningless for such data. You might also set the minimum a bit above zero, e.g. do BMI calculations really make sense for someone shorter than 50 cm?

 

[jmcilhinney]

I'm sorry if I've made myself sound stupid in this conversation but Ill take a look at the validation code you sent me as I understand how it would be more logical and efficient to do something like that.

There's no need to apologise. Just learn from your mistakes. Most people tend to be quite illogical and unsystematic when they start out and they make life harder for themselves as a result. We all want to just write code because that's the sexy part but the code we write tends to toward the garbage if we don't know what it's supposed to do, which few people actually take the time to work out to start with. The sooner you learn that lesson, the better for you.