Okay, so I have a main form which has a button for the user to add a new record. I have everything working except making sure the user enters appropriate values. I have it so a message box pops up the value is not valid, but it goes into an infinite loop. I am having a bitch of a time figuring out why. Here is my code:

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

namespace Record_Filer
{
    public partial class NewRecord : Form
    {
        public string Mame { set; get; }
        public string Genre { set; get; }
        public string Date { set; get; }
        public string Rating { set; get; }

        public NewRecord()
        {
            InitializeComponent();
        }

        private void NewRecord_Load(object sender, EventArgs e)
        {
            txbName.Text = "";
            txbGenre.Text = "";
            txbDate.Text = "";
            txbRating.Text = "";
            Mame = "";
            Genre = "";
            Date = "";
            Rating = "";
        }

        private void btnCancel_Click(object sender, EventArgs e)
        {
            txbName.Text = "";
            txbGenre.Text = "";
            txbDate.Text = "";
            txbRating.Text = "";
            Mame = "";
            Genre = "";
            Date = "";
            Rating = "";

            this.Hide();
        }

        private void btnSave_Click(object sender, EventArgs e)
        {
            int nameOK = 0;   //  selects the final closing of the process. ***not working correctly*** 
            int genreOK = 0;  //  maybe a switch would be better?
            int dateOK = 0;
            int rateOK = 0;
            int j = 0;

            while (j == 0)
            {
                if (txbName.Text != "")
                {
                    Mame = (txbName.Text); nameOK = 1;
                }
                else { MessageBox.Show("Please enter a DVD name"); nameOK = -1; }

                if (txbGenre.Text != "")
                {
                    Genre = (txbGenre.Text); genreOK = 1;
                }
                else { MessageBox.Show("Please enter a DVD genre"); genreOK = -1; }

                int date = 0;
                if (txbDate.Text != "")
                {
                    date = Convert.ToInt32(txbDate.Text);
                }
                if ((txbDate.Text != "") && (date >= 1800) && (date <= 3000))
                {
                    Date = (txbDate.Text); dateOK = 1;
                }
                else { MessageBox.Show("Please enter a DVD date between 1800 - 3000"); dateOK = -1; }

                int rate = 0;
                if (txbRating.Text != "")
                {
                    rate = Convert.ToInt32(txbRating.Text);
                }

                if ((txbRating.Text != "") && (rate >= 1) && (rate <= 5))
                {
                    Rating = (txbRating.Text); rateOK = 1;
                }
                else { MessageBox.Show("Please enter a DVD rating between 1 - 5"); rateOK = -1; }

                if ((nameOK == 1) && (dateOK == 1) && (genreOK == 1) && (rateOK == 1))
                {
                    j = 1; this.Close();   // ***** exits out and returns to main ... ****
                } 
            }
        }
    }
}

Any help on what I'm doing wrong here?

Recommended Answers

All 4 Replies

For what it's worth (I'm no pro) some advise:
Why do you use integers instead of booleans?
It feels much more natural to say nameOK = true than to say nameOK=1,-1 or whatever.
I would leave out the close on line 97. A save button has to do a save and nothing else.
Start a debug session and set some breakpoints, the VS debugger is really a marvel to use!
Succes!

For what it's worth (I'm no pro) some advise:
Why do you use integers instead of booleans?
It feels much more natural to say nameOK = true than to say nameOK=1,-1 or whatever.
I would leave out the close on line 97. A save button has to do a save and nothing else.
Start a debug session and set some breakpoints, the VS debugger is really a marvel to use!
Succes!

Well, I'm not the one who programmed that particular part. My partner did. I have no idea why he didn't use booleans. Thanks for the tips though!

You're entering an endless loop because of your while statement. You are setting j=0, then your while loop is based on while (j==0). If anything is incorrectly filled out then it shows the messageboxes, but it doesnt give the user a chance to change them. It gets to the end of the loop, something was wrong so j still equals 0 so the loop starts over again.
Take out the while loop and j. Its enough that at the end you only save if each of the other booleans is true.
Speaking of booleans, you dont need to explicitly check them.
If nameOK is set to 1 (which is true), when you test if(nameOK==1) its the same as saying if(true==true) . You can just use if(nameOK) to get the same result. If you needed to check for a false (eg if(nameOK != 1) || (nameOK == 0) ) you can use the NOT operator which is '!'; if(!nameOK) will only occur if nameOK is set to false.

if (nameOK  && dateOK && genreOK && rateOK)
                {
                    //shouldnt you 'save' something here??
                    this.Close();   // ***** exits out and returns to main ... **** 
                } 
                //if something was entered wrong then you wont save or exit and the user can change the values and try again.

You're entering an endless loop because of your while statement. You are setting j=0, then your while loop is based on while (j==0). If anything is incorrectly filled out then it shows the messageboxes, but it doesnt give the user a chance to change them. It gets to the end of the loop, something was wrong so j still equals 0 so the loop starts over again.
Take out the while loop and j. Its enough that at the end you only save if each of the other booleans is true.
Speaking of booleans, you dont need to explicitly check them.
If nameOK is set to 1 (which is true), when you test if(nameOK==1) its the same as saying if(true==true) . You can just use if(nameOK) to get the same result. If you needed to check for a false (eg if(nameOK != 1) || (nameOK == 0) ) you can use the NOT operator which is '!'; if(!nameOK) will only occur if nameOK is set to false.

if (nameOK  && dateOK && genreOK && rateOK)
                {
                    //shouldnt you 'save' something here??
                    this.Close();   // ***** exits out and returns to main ... **** 
                } 
                //if something was entered wrong then you wont save or exit and the user can change the values and try again.

Ah... thanks so much for the help. This was a pair programming assignment, by the way, I didn't just have somebody else do it for me.

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.