Before we can go and break something up to take our code to the next level. We need to clean up the complexity. This crazy code is massively complicated and it has to stop. Let’s simplify the logic and get ready so we can break it down later. Lets dig into how to go from a Noob to Pro by making your code less complicated.
Before we start her is the solution to yesterday’s problem.
In the setValue function, we have the results = “”. This does nothing so it can go away. I hope you found it.
First, lets break the values variables out of being the ones being built. I want to use a temp variable to take the input then let the setValue just determine which variable it will be assigned to in the system.
Noob to Pro Complicated Code
using System; using System.Windows.Forms; namespace NoobToPro { public partial class FormCalc : Form { string value1 = ""; string value2 = "" ; string results = "" ; string sign = ""; bool isValue2 = false; string temp = ""; public FormCalc() { InitializeComponent(); } private string setTemp(string number) { temp += number; return temp; } private string setValue(string number) { temp = ""; if (!isValue2) { value1 = number; return value1; } value2 = number; return value2; } private void setFunctionSwitchValuesAndClearInterface(string selectedSign) { sign = selectedSign; isValue2 = true; } private void btn7_Click(object sender, EventArgs e) { txtInput.Text = setTemp("7"); } private void btn8_Click(object sender, EventArgs e) { txtInput.Text = setTemp("8"); } private void btn9_Click(object sender, EventArgs e) { txtInput.Text = setTemp("9"); } private void btn4_Click(object sender, EventArgs e) { txtInput.Text = setTemp("4"); } private void btn5_Click(object sender, EventArgs e) { txtInput.Text = setTemp("5"); } private void btn6_Click(object sender, EventArgs e) { txtInput.Text = setTemp("6"); } private void btn1_Click(object sender, EventArgs e) { txtInput.Text = setTemp("1"); } private void btn2_Click(object sender, EventArgs e) { txtInput.Text = setTemp("2"); } private void btn3_Click(object sender, EventArgs e) { txtInput.Text = setTemp("3"); } private void btn0_Click(object sender, EventArgs e) { txtInput.Text = setTemp("0"); } private void btnPeriod_Click(object sender, EventArgs e) { txtInput.Text = setTemp("."); } private void btnDivide_Click(object sender, EventArgs e) { setValue(temp); setFunctionSwitchValuesAndClearInterface("/"); txtInput.Text = ""; } private void btnMultiply_Click(object sender, EventArgs e) { setValue(temp); setFunctionSwitchValuesAndClearInterface("*"); txtInput.Text = ""; } private void btnSubtract_Click(object sender, EventArgs e) { setValue(temp); setFunctionSwitchValuesAndClearInterface("-"); txtInput.Text = ""; } private void btnAdd_Click(object sender, EventArgs e) { setValue(temp); setFunctionSwitchValuesAndClearInterface("+"); txtInput.Text = ""; } private void btnEquals_Click(object sender, EventArgs e) { setValue(temp); switch (sign) { case "+": results = (Decimal.Parse(value1) + Decimal.Parse(value2)).ToString(); break; case "-": results = (Decimal.Parse(value1) - Decimal.Parse(value2)).ToString(); break; case "*": results = (Decimal.Parse(value1) * Decimal.Parse(value2)).ToString(); break; case "/": results = (Decimal.Parse(value1) / Decimal.Parse(value2)).ToString(); break; } value1 = ""; value2 = ""; txtInput.Text = value1; isValue2 = false; lblResult.Text = results; } private void FormCalc_Load(object sender, EventArgs e) { lblResult.Text = ""; } } }
First we create the temp variable, which will be our working variable. Next, we create the setTemp to handle the appending operation. Speaking of appending operation, we remove it from set value which now just sets the variable to the temp we drop in the parameter list.
After that we swap the setValue with setTemp for the input buttons and finally add the setValue function to the operator buttons. That wraps up step one.
Clean up on aisle 4
With that out of the way, we can focus on cleaning up some of our less desirable things for step 2. Notice the empty strings, we should get rid of those magic numbers and swap it out with string.Empty.
With our strings all emptied up, it’s time to look at our numbers. Keeping with the Noob to Pro, lets make them constants like you would see in a CompSci 101 class.
We will also follow suit with the operator signs.
Because we have just changed a bunch of stuff, lets look at where we are.
using System; using System.Windows.Forms; namespace NoobToPro { public partial class FormCalc : Form { string value1 = string.Empty; string value2 = string.Empty; string results = string.Empty; string sign = string.Empty; bool isValue2 = false; string temp = string.Empty; const string ONE = "1"; const string TWO = "2"; const string THREE = "3"; const string FOUR = "4"; const string FIVE = "5"; const string SIX = "6"; const string SEVEN = "7"; const string EIGHT = "8"; const string NINE = "9"; const string ZERO = "0"; const string PERIOD = "."; const string ADD = "+"; const string SUBTRACT = "-"; const string DIVIDE = "/"; const string MULTIPLE = "*"; public FormCalc() { InitializeComponent(); } private string setTemp(string number) { temp += number; return temp; } private string setValue(string number) { temp = string.Empty; if (!isValue2) { value1 = number; return value1; } value2 = number; return value2; } private void setFunctionSwitchValuesAndClearInterface(string selectedSign) { sign = selectedSign; isValue2 = true; } private void btn7_Click(object sender, EventArgs e) { txtInput.Text = setTemp(SEVEN); } private void btn8_Click(object sender, EventArgs e) { txtInput.Text = setTemp(EIGHT); } private void btn9_Click(object sender, EventArgs e) { txtInput.Text = setTemp(NINE); } private void btn4_Click(object sender, EventArgs e) { txtInput.Text = setTemp(FOUR); } private void btn5_Click(object sender, EventArgs e) { txtInput.Text = setTemp(FIVE); } private void btn6_Click(object sender, EventArgs e) { txtInput.Text = setTemp(SIX); } private void btn1_Click(object sender, EventArgs e) { txtInput.Text = setTemp(ONE); } private void btn2_Click(object sender, EventArgs e) { txtInput.Text = setTemp(TWO); } private void btn3_Click(object sender, EventArgs e) { txtInput.Text = setTemp(THREE); } private void btn0_Click(object sender, EventArgs e) { txtInput.Text = setTemp(ZERO); } private void btnPeriod_Click(object sender, EventArgs e) { txtInput.Text = setTemp(PERIOD); } private void btnDivide_Click(object sender, EventArgs e) { setValue(temp); setFunctionSwitchValuesAndClearInterface(DIVIDE); txtInput.Text = string.Empty; } private void btnMultiply_Click(object sender, EventArgs e) { setValue(temp); setFunctionSwitchValuesAndClearInterface(MULTIPLE); txtInput.Text = string.Empty; } private void btnSubtract_Click(object sender, EventArgs e) { setValue(temp); setFunctionSwitchValuesAndClearInterface(SUBTRACT); txtInput.Text = string.Empty; } private void btnAdd_Click(object sender, EventArgs e) { setValue(temp); setFunctionSwitchValuesAndClearInterface(ADD); txtInput.Text = string.Empty; } private void btnEquals_Click(object sender, EventArgs e) { setValue(temp); switch (sign) { case ADD: results = (Decimal.Parse(value1) + Decimal.Parse(value2)).ToString(); break; case SUBTRACT: results = (Decimal.Parse(value1) - Decimal.Parse(value2)).ToString(); break; case MULTIPLE: results = (Decimal.Parse(value1) * Decimal.Parse(value2)).ToString(); break; case DIVIDE: results = (Decimal.Parse(value1) / Decimal.Parse(value2)).ToString(); break; } value1 = string.Empty; value2 = string.Empty; txtInput.Text = value1; isValue2 = false; lblResult.Text = results; } private void FormCalc_Load(object sender, EventArgs e) { lblResult.Text = string.Empty; } } }
Keep our functions focused
Before we go and wrap this one up, lets go through one more sweep and see how things look. One thing I don’t like is the setValue is both setting the value and clearing the temp string. I will move that out of there and have it put back into the operator click events.
With that done, our setValue justs figure out how to set the value. I really hate the setFunctionSwitchValueAndClearInterface function. Not only is it a mouthful but it is also an eye sore. Let’s see if we can fix that.
First the sign=selectedSign and isValue2 true are two separate things. Lets treat them as such. Next the sign setting will be put into the operating button click events. Now that we moved the append function out of the setValue, we can put the setting of value items into it entirely. So lets move it into the setValue after we set value 1.
After all that we do have some duplication in the operator button click events. I am sure some of you are thinking, we undid things we did in previous posts. If you thought that, you are right. We applied a new rule on top of the don’t repeat we did before. We wanted functions that do one thing only. So things got a bit undone as a result.
In the operation button’s click events we have it setting the value, clearing a system variable, setting a system variable and clearing out a GUI element. These are basically several different functions and bundling them would violate keep methods focused on one thing. As a result, I will leave them be and move on to my next series of improvements.
After all that we still have a major sin, we have mixed our GUI and business logic together. This must be cleaned up to move to the next level from Noob to Pro. We will look at that in the next post.
Even with that sin, you can compare where we started and where we are now and things just look and feel cleaner and simpler. It’s much less complicated a big step from going from a Noob to Pro.
Here is where we are now. As always leave your questions or comments down below.
Noob to Pro Less Complicated FormCalc
using System; using System.Windows.Forms; namespace NoobToPro { public partial class FormCalc : Form { string value1 = string.Empty; string value2 = string.Empty; string results = string.Empty; string sign = string.Empty; bool isValue2 = false; string temp = string.Empty; const string ONE = "1"; const string TWO = "2"; const string THREE = "3"; const string FOUR = "4"; const string FIVE = "5"; const string SIX = "6"; const string SEVEN = "7"; const string EIGHT = "8"; const string NINE = "9"; const string ZERO = "0"; const string PERIOD = "."; const string ADD = "+"; const string SUBTRACT = "-"; const string DIVIDE = "/"; const string MULTIPLE = "*"; public FormCalc() { InitializeComponent(); } private string setTemp(string number) { temp += number; return temp; } private string setValue(string number) { if (!isValue2) { value1 = number; isValue2 = true; return value1; } value2 = number; return value2; } private void btn7_Click(object sender, EventArgs e) { txtInput.Text = setTemp(SEVEN); } private void btn8_Click(object sender, EventArgs e) { txtInput.Text = setTemp(EIGHT); } private void btn9_Click(object sender, EventArgs e) { txtInput.Text = setTemp(NINE); } private void btn4_Click(object sender, EventArgs e) { txtInput.Text = setTemp(FOUR); } private void btn5_Click(object sender, EventArgs e) { txtInput.Text = setTemp(FIVE); } private void btn6_Click(object sender, EventArgs e) { txtInput.Text = setTemp(SIX); } private void btn1_Click(object sender, EventArgs e) { txtInput.Text = setTemp(ONE); } private void btn2_Click(object sender, EventArgs e) { txtInput.Text = setTemp(TWO); } private void btn3_Click(object sender, EventArgs e) { txtInput.Text = setTemp(THREE); } private void btn0_Click(object sender, EventArgs e) { txtInput.Text = setTemp(ZERO); } private void btnPeriod_Click(object sender, EventArgs e) { txtInput.Text = setTemp(PERIOD); } private void btnDivide_Click(object sender, EventArgs e) { setValue(temp); temp = string.Empty; sign = DIVIDE; txtInput.Text = string.Empty; } private void btnMultiply_Click(object sender, EventArgs e) { setValue(temp); temp = string.Empty; sign = MULTIPLE; txtInput.Text = string.Empty; } private void btnSubtract_Click(object sender, EventArgs e) { setValue(temp); temp = string.Empty; sign = SUBTRACT; txtInput.Text = string.Empty; } private void btnAdd_Click(object sender, EventArgs e) { setValue(temp); temp = string.Empty; sign = ADD; txtInput.Text = string.Empty; } private void btnEquals_Click(object sender, EventArgs e) { setValue(temp); temp = string.Empty; switch (sign) { case ADD: results = (Decimal.Parse(value1) + Decimal.Parse(value2)).ToString(); break; case SUBTRACT: results = (Decimal.Parse(value1) - Decimal.Parse(value2)).ToString(); break; case MULTIPLE: results = (Decimal.Parse(value1) * Decimal.Parse(value2)).ToString(); break; case DIVIDE: results = (Decimal.Parse(value1) / Decimal.Parse(value2)).ToString(); break; } value1 = string.Empty; value2 = string.Empty; txtInput.Text = value1; isValue2 = false; lblResult.Text = results; } private void FormCalc_Load(object sender, EventArgs e) { lblResult.Text = string.Empty; } } }