Disable and enable text boxes according to the radio button click

Question

Here is a GPA calculator in C#. I am feeling like the code that I write and format are quite long and would like it to be shorter. This program is using radio buttons to enable and disable the text boxes according to the number of subject chosen. Perhaps that part can be shortened.

public double gradePoint1, gradePoint2, gradePoint3, gradePoint4, gradePoint5, gradePoint6;
public double courseGP1, courseGP2, courseGP3, courseGP4, courseGP5, courseGP6;
int credHour1 = 0, credHour2 = 0, credHour3 = 0, credHour4 = 0, credHour5 = 0, credHour6 = 0;
public Form1()
{
 InitializeComponent();
}
private void Form1_Load(object sender, EventArgs e)
{
 // make the display bigger
 this.Font = new Font("Arial", 15);
}
private void btnDisplay_Click(object sender, EventArgs e)
{
 // to disable certain text boxes according to the number of courses taken
 if (rad1.Checked)
 DisplayOne();
 else if (rad2.Checked)
 DisplayTwo();
 else if (rad3.Checked)
 DisplayThree();
 else if (rad4.Checked)
 DisplayFour();
 else if (rad5.Checked)
 DisplayFive();
 else
 DisplaySix();
 InitializeLabels();
}
private void InitializeLabels()
{
 // initialize all Credit Hours labels
 lblCH1.Text = "0";
 lblCH2.Text = "0";
 lblCH3.Text = "0";
 lblCH4.Text = "0";
 lblCH5.Text = "0";
 lblCH6.Text = "0";
}
private void DisplayOne()
{
 txtCCode1.Enabled = true;
 txtCCode2.Enabled = false;
 txtCCode3.Enabled = false;
 txtCCode4.Enabled = false;
 txtCCode5.Enabled = false;
 txtCCode6.Enabled = false;
 txtGrade1.Enabled = true;
 txtGrade2.Enabled = false;
 txtGrade3.Enabled = false;
 txtGrade4.Enabled = false;
 txtGrade5.Enabled = false;
 txtGrade6.Enabled = false;
}
private void DisplayTwo()
{
 txtCCode1.Enabled = true;
 txtCCode2.Enabled = true;
 txtCCode3.Enabled = false;
 txtCCode4.Enabled = false;
 txtCCode5.Enabled = false;
 txtCCode6.Enabled = false;
 txtGrade1.Enabled = true;
 txtGrade2.Enabled = true;
 txtGrade3.Enabled = false;
 txtGrade4.Enabled = false;
 txtGrade5.Enabled = false;
 txtGrade6.Enabled = false;
}
private void DisplayThree()
{
 txtCCode1.Enabled = true;
 txtCCode2.Enabled = true;
 txtCCode3.Enabled = true;
 txtCCode4.Enabled = false;
 txtCCode5.Enabled = false;
 txtCCode6.Enabled = false;
 txtGrade1.Enabled = true;
 txtGrade2.Enabled = true;
 txtGrade3.Enabled = true;
 txtGrade4.Enabled = false;
 txtGrade5.Enabled = false;
 txtGrade6.Enabled = false;
}
private void DisplayFour()
{
 txtCCode1.Enabled = true;
 txtCCode2.Enabled = true;
 txtCCode3.Enabled = true;
 txtCCode4.Enabled = true;
 txtCCode5.Enabled = false;
 txtCCode6.Enabled = false;
 txtGrade1.Enabled = true;
 txtGrade2.Enabled = true;
 txtGrade3.Enabled = true;
 txtGrade4.Enabled = true;
 txtGrade5.Enabled = false;
 txtGrade6.Enabled = false;
}
private void DisplayFive()
{
 txtCCode1.Enabled = true;
 txtCCode2.Enabled = true;
 txtCCode3.Enabled = true;
 txtCCode4.Enabled = true;
 txtCCode5.Enabled = true;
 txtCCode6.Hide();
 txtGrade1.Enabled = true;
 txtGrade2.Enabled = true;
 txtGrade3.Enabled = true;
 txtGrade4.Enabled = true;
 txtGrade5.Enabled = true;
 txtGrade6.Hide();
 lblCH6.Hide();
}
private void DisplaySix()
{
 txtCCode1.Enabled = true;
 txtCCode2.Enabled = true;
 txtCCode3.Enabled = true;
 txtCCode4.Enabled = true;
 txtCCode5.Enabled = true;
 txtCCode6.Enabled = true;
 txtGrade1.Enabled = true;
 txtGrade2.Enabled = true;
 txtGrade3.Enabled = true;
 txtGrade4.Enabled = true;
 txtGrade5.Enabled = true;
 txtGrade6.Enabled = true;
 txtCCode6.Show();
 txtGrade6.Show();
 lblCH6.Show();
}
private void txtCCode1_Leave(object sender, EventArgs e)
{
 // to find the right most character from the course code = CREDIT HOURS
 string courseCode = txtCCode1.Text;
 string rightMost = courseCode.Right(1);
 lblCH1.Text = rightMost;
 credHour1 = Convert.ToInt32(lblCH1.Text);
}
private void txtCCode2_Leave(object sender, EventArgs e)
{
 string courseCode = txtCCode2.Text;
 string rightMost = courseCode.Right(1);
 lblCH2.Text = rightMost;
 credHour2 = Convert.ToInt32(lblCH2.Text);
}
private void txtCCode3_Leave(object sender, EventArgs e)
{
 string courseCode = txtCCode3.Text;
 string rightMost = courseCode.Right(1);
 lblCH3.Text = rightMost;
 credHour3 = Convert.ToInt32(lblCH3.Text);
}
private void txtCCode4_Leave(object sender, EventArgs e)
{
 string courseCode = txtCCode4.Text;
 string rightMost = courseCode.Right(1);
 lblCH4.Text = rightMost;
 credHour4 = Convert.ToInt32(lblCH4.Text);
}
private void txtCCode5_Leave(object sender, EventArgs e)
{
 string courseCode = txtCCode5.Text;
 string rightMost = courseCode.Right(1);
 lblCH5.Text = rightMost;
 credHour5 = Convert.ToInt32(lblCH5.Text);
}
private void txtCCode6_Leave(object sender, EventArgs e)
{
 string courseCode = txtCCode6.Text;
 string rightMost = courseCode.Right(1);
 lblCH6.Text = rightMost;
 credHour6 = Convert.ToInt32(lblCH6.Text);
}
private void txtGrade1_Leave(object sender, EventArgs e)
{
 gradePoint1 = FindGradePoint(txtGrade1.Text);
}
private void txtGrade2_Leave(object sender, EventArgs e)
{
 gradePoint2 = FindGradePoint(txtGrade2.Text);
}
private void txtGrade3_Leave(object sender, EventArgs e)
{
 gradePoint3 = FindGradePoint(txtGrade3.Text);
}
private void txtGrade4_Leave(object sender, EventArgs e)
{
 gradePoint4 = FindGradePoint(txtGrade4.Text);
}
private void txtGrade5_Leave(object sender, EventArgs e)
{
 gradePoint5 = FindGradePoint(txtGrade5.Text);
}
private void txtGrade6_Leave(object sender, EventArgs e)
{
 gradePoint6 = FindGradePoint(txtGrade6.Text);
}
private double FindGradePoint(string grade)
{
 double gradePt=0;
 switch (grade)
 {
 case "A+":
 case "A":
 gradePt = 4.00;
 break;
 case "A-":
 gradePt = 3.75;
 break;
 case "B+":
 gradePt = 3.5;
 break;
 case "B":
 gradePt = 3.0;
 break;
 case "B-":
 gradePt = 2.75;
 break;
 case "C+":
 gradePt = 2.5;
 break;
 case "C":
 gradePt = 2.0;
 break;
 case "C-":
 gradePt = 1.75;
 break;
 case "D+":
 gradePt = 1.5;
 break;
 case "D":
 gradePt = 1.0;
 break;
 case "E":
 gradePt = 0.5;
 break;
 case "F":
 gradePt = 0.0;
 break;
 default:
 DialogResult x = new DialogResult();
 x = MessageBox.Show("Error in Alphabet grade", "Error");
 if (x == DialogResult.OK)
 txtGrade1.Focus();
 break;
 }
 return gradePt;
}
private void button1_Click(object sender, EventArgs e)
{
 int totalCredHours = 0;
 CalcTotalCredHours(credHour1, credHour2, credHour3, credHour4, credHour5, credHour6, ref totalCredHours);
 courseGP1 = CalcCourseGradePoint(credHour1, gradePoint1);
 courseGP2 = CalcCourseGradePoint(credHour2, gradePoint2);
 courseGP3 = CalcCourseGradePoint(credHour3, gradePoint3);
 courseGP4 = CalcCourseGradePoint(credHour4, gradePoint4);
 courseGP5 = CalcCourseGradePoint(credHour5, gradePoint5);
 courseGP6 = CalcCourseGradePoint(credHour6, gradePoint6);
 double totalCGP = CalcTotalCGP(courseGP1, courseGP2, courseGP3, courseGP4, courseGP5, courseGP6);
 double gpa = CalcGPA(totalCGP, totalCredHours);
 lblGPA.Text = gpa.ToString("N");
}
private double CalcCourseGradePoint(int ch, double gp)
{
 double cgp = ch * gp;
 return cgp;
}
private double CalcGPA(double tcgp, int tch)
{
 double gpa = tcgp / tch;
 return gpa;
}
private double CalcTotalCGP(double cgp1, double cgp2, double cgp3, double cgp4, double cgp5, double cgp6)
{
 double totCGP = cgp1 + cgp2 + cgp3 + cgp4 + cgp5 + cgp6;
 return totCGP;
}
private void CalcTotalCredHours(int ch1, int ch2, int ch3, int ch4, int ch5, int ch6, ref int tch)
{
 tch = ch1 + ch2 + ch3 + ch4 + ch5 + ch6;
}
private void textBox1_TextChanged(object sender, EventArgs e)
{
}
private void textBox1_DoubleClick(object sender, EventArgs e)
{
 textBox2.Text = textBox1.Text.Right(4).ToLower();
}
}

• Bjørn-Roger Kringsjå: I see no radio button click event handlers.

Answer 1

There are quite a few issues in this code that would make it unsuitable for production/commercial use and I'll run through what we've got. Since this appears to be a Homework assignment I will only provide a small number of code samples for you, it will be up to you to implement the advice into your code.

public double gradePoint1, gradePoint2, gradePoint3, gradePoint4, gradePoint5, gradePoint6;
public double courseGP1, courseGP2, courseGP3, courseGP4, courseGP5, courseGP6;
int credHour1 = 0, credHour2 = 0, credHour3 = 0, credHour4 = 0, credHour5 = 0, credHour6 = 0;

Declaring multiple single variables is bad. There are better ways to represent your data in C#, for example look at using struct to structure your data entries and an Array or List to hold them for processing in place of defining each variable individually.

The variable names are not very meaningful. When this happens it should be taken as a sign that you may have started writing code before you've fully understood the problem.

public Form1()

Shows us there is a mix of 'business logic' (or the rules of what need to be processed and how) and 'user interface' code. While it was once common practice in winforms code to do this, it's a bad habit that you shouldn't get into. Any code related to the processing of data should be moved out of this class into a separate file and class.

Moving the business logic to another class also means that you can test your code more easily and can write automated tests that ensure any changes later don't break something that previously worked.

For example you can create a console app which feeds the class known good (and bad) data to make sure that you get the results you expect.

this.Font = new Font("Arial", 15);

Is in the wrong place as you can set the default font & size for a form via the forms designer.

private void btnDisplay_Click(object sender, EventArgs e)
{
 // to disable certain text boxes according to the number of courses taken
 if (rad1.Checked)
 ...

The naming of this code makes me think that after selecting a radio button that users need to click a button to change the interface to allow them to enter the correct data. Check the radio button events and you'll find that they too have a 'click' event which can enable changes to the UI as you select an option.

Radio buttons in general are not especially useful in this context since users can just leave rows blank if they are not needed.

rad1 is not an especially useful name. It doesn't tell someone reviewing the code what the radio button is for.

lblCH1.Text = "0";
lblCH2.Text = "0";
lblCH3.Text = "0";

Is a situation where you could look at using something called a UserControl and a Control Array; these would allow you to stop repeating large chunks of code with minor changes as you are now.

In fact, comparing DisplayOne() and DisplayFive() reveals a UI bug. If you run DisplayFive() then we hit the line of code txtCCode6.Hide(); which removes the box entirely but since there is no txtCCode6.Show(); to restore the textbox, the user will never be able to use it. Take this code...

private void EnableFields()
{
 txtCCode6.Enabled = rad6.Checked;
 txtGrade6.Enabled = txtCCode6.Enabled;
 // we need to enable txt5 if txt6 is enabled OR 
 txtCCode5.Enabled = rad5.Checked || txtCCode6.Enabled;
 txtGrade5.Enabled = txtCCode5.Enabled;
 ...

Using code like this we can get rid of all the DisplayOne, DisplayTwo functions and their repeated code and combine them all into one. Whenever the number of entries is set, only one set of code is run and they can't get out of sync.

private double FindGradePoint(string grade)
{
 double gradePt=0;
 switch (grade)
 {
 case "A+":
 case "A":
 ...

Could be shortened extensively using either a Dictionary<string,float> or using an enum with a value and the Description attribute.

There is quite a bit more that you could do, but what's here will be plenty for now...

• RobH: I want to +1 but I disagree so strongly with the advice to create a struct that I can't bring myself to do it. If you recommend a class instead, you'll have my +1.
• James Snell: class vs struct depends on what part of the code you're looking at and there's some good lessons to be learned from knowing when to use which. I'll edit to point out that that the business logic should go into a class but the scores from the array/list are more correct as a struct since it will only store reference types and return a calculated value.
• Evan Skull: Thank you for your answer. I will look through the things that you said.