Hi everyone, i just started learning c# in school and i need some help cleaning up my code as its very messy and i don’t know how to do it. i have a lot of code that is reused and i tried moving it to public but the code didn’t work after that. How can i easily reuse the code without copy pasting in every button?
private void managerPlusButton_Click(object sender, EventArgs e)
{
managerInteger++;
managerTextBox.Text = managerInteger.ToString();
managerSalaryInterger = managerInteger * 1000;
managerSalaryTextBox.Text = managerSalaryInterger.ToString(“C”);
totalInteger = chefInteger + serverInteger + dishwasherInteger + managerInteger + cleanerInteger + cashierInteger;
totalTextBox.Text = totalInteger.ToString();
totalSalaryInteger = chefSalaryInteger + serverSalaryInteger + dishwasherSalaryInteger + managerSalaryInterger + cleanerSalaryInteger + cashierSalaryInteger;
totalSalaryTextBox.Text = totalSalaryInteger.ToString(“C”);
}
private void managerMinusButton_Click(object sender, EventArgs e)
{
managerInteger–;
managerTextBox.Text = managerInteger.ToString();
managerSalaryInterger = managerInteger * 1000;
managerSalaryTextBox.Text = managerSalaryInterger.ToString(“C”);
totalInteger = chefInteger + serverInteger + dishwasherInteger + managerInteger + cleanerInteger + cashierInteger;
totalTextBox.Text = totalInteger.ToString();
totalSalaryInteger = chefSalaryInteger + serverSalaryInteger + dishwasherSalaryInteger + managerSalaryInterger + cleanerSalaryInteger + cashierSalaryInteger;
totalSalaryTextBox.Text = totalSalaryInteger.ToString(“C”);
}
Hi, and yes it is a bit messy 🙂
First of all, I do not judge you. You have just started to learn to code and this is OK to make a mess 🙂
This what you need but this is the same mess as it was.
private void managerPlusButton_Click(object sender, EventArgs e) {
managerInteger++;
UpdateSalary(managerInteger);
}
private void managerMinusButton_Click(object sender, EventArgs e) {
managerInteger–;
UpdateSalary(managerInteger);
}
void UpdateSalary(int managerInt) {
managerTextBox.Text = managerInt.ToString();
managerSalaryInterger = managerInt * 1000;
managerSalaryTextBox.Text = managerSalaryInterger.ToString(“C”);
totalInteger = chefInteger + serverInteger + dishwasherInteger + managerInt + cleanerInteger + cashierInteger;
totalTextBox.Text = totalInteger.ToString();
totalSalaryInteger = chefSalaryInteger + serverSalaryInteger + dishwasherSalaryInteger + managerSalaryInterger + cleanerSalaryInteger + cashierSalaryInteger;
totalSalaryTextBox.Text = totalSalaryInteger.ToString(“C”);
}
To continue cleaning you have to do better naming and apply OOP approaches.
Your going to get a wild variety of answers here.
Your doing a lot of logic in what looks like an event (are you using Winforms or WPF?). Technically you shouldn’t be doing any logic that doesnt involve the ViewModel in the events (This is standard practice to prevent cluttering UI.) What you could do is move this code to a new public static class and call methods from it instead. You can call methods from anywhere that’s public it doesn’t have to be the main window/form.
It’s hard to recommend how to give you a better format until you understand more about coding, data structures, and standard practice. The more you learn and practice the more you’ll piece together how to format your code better.
Edit: For a quick and dirty way:
Since you want to reuse your code for multiple buttons you can make a new method in your mainform/mainwindow
Private static void CodeForMyButtons(){ Paste code here }
And then where you have your current code (in the events) you could say CodeForMyButtons()
First thing that jumps at me is variable naming. Please drop the type name from variable names. Just to give a few reasons: It is rarely useful, it can steal focus from the actual program logic, and can actually lead to bugs if you change the type but forget to change the name.
Additionally, I don’t think a salary should be stored in an integer variable.
Method names should begin with a capital letter. (PascalCase)
If this is WPF, you may be able to clean up considerably by using bindings in the controls.
So, in the textbox, you say Text={Binding managerSalary}.
Now, in your buttonClick, you’d only need to do managerSalary++. The framework takes care of updating the UI.