After 9 posts, we have taken our calculator far in the world. In this last one, we put some spit polish on what we have. In addition to that polish, we will do a little data validation and try to make it a bit less breaky in general.
Final Cleanup GUI
Here is the GUI as it currently stands.
We still have a problem. Some jackass could type Hi Mom into the textbox and blowup everything.
We could wrap everything in try catches or do one of the many ways to check the data type. Notice that the error is the error we made to throw when a non number was found. As a result, this is a GUI level problem. Our code handles it as we intended.
The most elegant and simplistic approach to solving the problem is to make the box read only. So look into the properties and set to read only. Now the only way the text gets in that box is through our buttons.
This means there is now only one invalid combination to deal with and that is a lone period. Lets put the fix in for that. Because a number and period equals a number we only have one scenario to worry about here. In the btnPeriod_click change it by adding a check.
private void btnPeriod_Click(object sender, EventArgs e) { txtInput.Text += PERIOD; if(txtInput.Text.Length == 1) { txtInput.Text = "0."; } }
That is a valid solution that would work but lets fix it closer to the source so that if we port this to a web calculator or something we retain that protection.
The easiest way to work this is to go back to your unit tests and add a new TestCase so in the FormCalcController go add a new TestCase for period with a expected result of 0.
[TestCase("1.0", 1f)] [TestCase("1.", 1f) [TestCase(".", 0f)] public void setAddition_test(string inputValue, float expectedValue) { //arrange //act fcc.setAddition(inputValue); //assert Assert.IsTrue(fcc.values.First().sign == "+"); Assert.AreEqual(fcc.values.First().value, expectedValue); }
Your test case will fail. So we have our failure. Now we need the correction. However there is another problem. If you look closely you will see we need to deal with the Number class. So lets deal with it.
[TestCase("5.6", 5.6f)] [TestCase(".", 0f)] public void ParseSetValue_Test_AreEqual(string valueToParse, float expectedValue) { //arrange -done in setup //act number.ParseSetValue(valueToParse); //assert Assert.AreEqual(expectedValue, number.value); }
So we have two problems. The first how to solve a problem and another on how to prevent a derived unit test that missed its target. The second one is implementing mocks which is a bit outside where I want to take this series.
The solution to the first is to remove our throws exception test in the Number_Tests the single period. Then in the ParseSetValue run a check for a period and correct. While we are at it, we should deal with multiple periods as well. The first is easy.
using System; namespace NoobToPro { public class Number { public float value { get; set; } public string sign { get; set; } public void ParseSetValue(string valueToParse) { bool parseSuccess; float temp; if (valueToParse.Trim() == ".") { valueToParse = "0."; } parseSuccess = float.TryParse(valueToParse, out temp); if (parseSuccess) { value = temp; } else { throw new Exception("Value was not a number."); } } } }
The next is a GUI thing to fix. It is also stupid simple to fix.
private void btnPeriod_Click(object sender, EventArgs e) { if (!txtInput.Text.Contains(".")) { txtInput.Text += PERIOD; } }
We just check the output for existing periods. If none are present then add, otherwise ignore.
With that out of the way lets deal with the double clicks of any of the operators. This causes an error. Again this will be a invalid value that we threw so it is not an error in the code. It is something intending the GUI to handle. So lets handle it through checking the txtInput length.
private void btnDivide_Click(object sender, EventArgs e) { if(txtInput.Text.Trim().Length > 0) { controller.setDivide(txtInput.Text); txtInput.Text = string.Empty; } } private void btnMultiply_Click(object sender, EventArgs e) { if (txtInput.Text.Trim().Length > 0) { controller.setMultiply(txtInput.Text); txtInput.Text = string.Empty; } } private void btnSubtract_Click(object sender, EventArgs e) { if (txtInput.Text.Trim().Length > 0) { controller.setSubtract(txtInput.Text); txtInput.Text = string.Empty; } } private void btnAdd_Click(object sender, EventArgs e) { if (txtInput.Text.Trim().Length > 0) { controller.setAddition(txtInput.Text); txtInput.Text = string.Empty; } } private void btnEquals_Click(object sender, EventArgs e) { if (txtInput.Text.Trim().Length > 0) { lblResult.Text = controller.setEquals(txtInput.Text).ToString(); txtInput.Text = string.Empty; } }
So we got some last integration testing done and everything is looking fine.
We still need to purge the magic strings we introduced in our validation. As well as all the using directives we are not using and the commented out code. Lets see where we landed.
FormCalc.cs
using System; using System.Windows.Forms; namespace NoobToPro { public partial class FormCalc : Form { FormCalcController controller = new FormCalcController(); 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 = "*"; const string EQUALS = "="; public FormCalc() { InitializeComponent(); } private void btn7_Click(object sender, EventArgs e) { txtInput.Text += SEVEN; } private void btn8_Click(object sender, EventArgs e) { txtInput.Text += EIGHT; } private void btn9_Click(object sender, EventArgs e) { txtInput.Text += NINE; } private void btn4_Click(object sender, EventArgs e) { txtInput.Text += FOUR; } private void btn5_Click(object sender, EventArgs e) { txtInput.Text += FIVE; } private void btn6_Click(object sender, EventArgs e) { txtInput.Text += SIX; } private void btn1_Click(object sender, EventArgs e) { txtInput.Text += ONE; } private void btn2_Click(object sender, EventArgs e) { txtInput.Text += TWO; } private void btn3_Click(object sender, EventArgs e) { txtInput.Text += THREE; } private void btn0_Click(object sender, EventArgs e) { txtInput.Text += ZERO; } private void btnPeriod_Click(object sender, EventArgs e) { if (!txtInput.Text.Contains(PERIOD)) { txtInput.Text += PERIOD; } } private void btnDivide_Click(object sender, EventArgs e) { if(txtInput.Text.Trim().Length > 0) { controller.setDivide(txtInput.Text); txtInput.Text = string.Empty; } } private void btnMultiply_Click(object sender, EventArgs e) { if (txtInput.Text.Trim().Length > 0) { controller.setMultiply(txtInput.Text); txtInput.Text = string.Empty; } } private void btnSubtract_Click(object sender, EventArgs e) { if (txtInput.Text.Trim().Length > 0) { controller.setSubtract(txtInput.Text); txtInput.Text = string.Empty; } } private void btnAdd_Click(object sender, EventArgs e) { if (txtInput.Text.Trim().Length > 0) { controller.setAddition(txtInput.Text); txtInput.Text = string.Empty; } } private void btnEquals_Click(object sender, EventArgs e) { if (txtInput.Text.Trim().Length > 0) { lblResult.Text = controller.setEquals(txtInput.Text).ToString(); txtInput.Text = string.Empty; } } private void FormCalc_Load(object sender, EventArgs e) { lblResult.Text = string.Empty; } } }
FormCalcController.cs
using System.Collections.Generic; namespace NoobToPro { public class FormCalcController { public List<Number> values { get; set; } const string ADD = "+"; const string SUBTRACT = "-"; const string DIVIDE = "/"; const string MULTIPLY = "*"; public FormCalcController() { values = new List<Number>(); } public void setAddition(string inputValue) { Number temp = new Number(); temp.sign = ADD; temp.ParseSetValue(inputValue); values.Add(temp); } public void setSubtract(string inputValue) { Number temp = new Number(); temp.sign = SUBTRACT; temp.ParseSetValue(inputValue); values.Add(temp); } public void setMultiply(string inputValue) { Number temp = new Number(); temp.sign = MULTIPLY; temp.ParseSetValue(inputValue); values.Add(temp); } public void setDivide(string inputValue) { Number temp = new Number(); temp.sign = DIVIDE; temp.ParseSetValue(inputValue); values.Add(temp); } public float setEquals(string finalValue) { string lastSign; Number final = new Number(); Number temp = new Number(); temp.ParseSetValue(finalValue); values.Add(temp); if (values.Count >= 1) { final = values[0]; values.RemoveAt(0); lastSign = final.sign; foreach (Number val in values) { switch (lastSign) { case ADD: final.value += val.value; break; case SUBTRACT: final.value -= val.value; break; case MULTIPLY: final.value *= val.value; break; case DIVIDE: final.value /= val.value; break; } lastSign = val.sign; } } values = new List<Number>(); lastSign = string.Empty; return final.value; } } }
Number.cs
using System; namespace NoobToPro { public class Number { public float value { get; set; } public string sign { get; set; } const string PERIOD = "."; const string PERIOD_REPLACEMENT = "0."; const string FAILED_PARSE_MESSAGE = "Value was not a number."; public void ParseSetValue(string valueToParse) { bool parseSuccess; float temp; if (valueToParse.Trim() == PERIOD) { valueToParse = PERIOD_REPLACEMENT; } parseSuccess = float.TryParse(valueToParse, out temp); if (parseSuccess) { value = temp; } else { throw new Exception(FAILED_PARSE_MESSAGE); } } } }
FormCalcController_Tests
using System.Linq; using NUnit.Framework; using NoobToPro; namespace NoobToPro_Tests { [TestFixture] public class FormCalcController_Tests { FormCalcController fcc; [SetUp] public void SetUp() { fcc = new FormCalcController(); } [TestCase("1.0", 1f)] [TestCase("1.", 1f)] [TestCase(".", 0f)] public void setAddition_test(string inputValue, float expectedValue) { //arrange //act fcc.setAddition(inputValue); //assert Assert.IsTrue(fcc.values.First().sign == "+"); Assert.AreEqual(fcc.values.First().value, expectedValue); } [TestCase("1.0", 1f)] public void setSubtract_test(string inputValue, float expectedValue) { //arrange //act fcc.setSubtract(inputValue); //assert Assert.IsTrue(fcc.values.First().sign == "-"); Assert.AreEqual(fcc.values.First().value, expectedValue); } [TestCase("1.0", 1f)] public void setMultiply_test(string inputValue, float expectedValue) { //arrange //act fcc.setMultiply(inputValue); //assert Assert.IsTrue(fcc.values.First().sign == "*"); Assert.AreEqual(fcc.values.First().value, expectedValue); } [TestCase("1.0", 1f)] public void setDivision_test(string inputValue, float expectedValue) { //arrange //act fcc.setDivide(inputValue); //assert Assert.IsTrue(fcc.values.First().sign == "/"); Assert.AreEqual(fcc.values.First().value, expectedValue); } [TestCase("3.5", "4", "5.0", 12.5f)] public void runEquation_sum_Test(string value1, string value2, string value3, float expectedResult) { //arrange float result; fcc.setAddition(value1); fcc.setAddition(value2); //act result = fcc.setEquals(value3); //assert Assert.AreEqual(expectedResult, result); } [TestCase("3.5", "4", "5.0", -5.5f)] public void runEquation_difference_Test(string value1, string value2, string value3, float expectedResult) { //arrange float result; fcc.setSubtract(value1); fcc.setSubtract(value2); //act result = fcc.setEquals(value3); //assert Assert.AreEqual(expectedResult, result); } [TestCase("3.5", "4", "5.0", 70f)] public void runEquation_product_Test(string value1, string value2, string value3, float expectedResult) { //arrange float result; fcc.setMultiply(value1); fcc.setMultiply(value2); //act result = fcc.setEquals(value3); //assert Assert.AreEqual(expectedResult, result); } [TestCase("3.5", "4", "5.0", 0.175f)] public void runEquation_quotient_Test(string value1, string value2, string value3, float expectedResult) { //arrange float result; fcc.setDivide(value1); fcc.setDivide(value2); //act result = fcc.setEquals(value3); //assert Assert.AreEqual(expectedResult, result); } [TestCase("3.5", "4", "5.0", "1.9", 4.75f)] public void runEquation_multi_Add_Sub_Mult_Test(string value1, string value2, string value3, string value4, float expectedResult) { //arrange float result; fcc.setAddition(value1); fcc.setSubtract(value2); fcc.setMultiply(value3); //act result = fcc.setEquals(value4); //assert Assert.AreEqual(expectedResult, result); } [TearDown] public void TearDown() { fcc = null; } } }
Number_Tests.cs
using System; using NUnit.Framework; using NoobToPro; namespace NoobToPro_Tests { [TestFixture] public class Number_Tests { Number number; [SetUp] public void Setup() { number = new Number(); } [TestCase("1", 1f)] [TestCase("2", 2f)] [TestCase("3", 3f)] [TestCase("4", 4f)] [TestCase("5", 5f)] [TestCase("6", 6f)] [TestCase("7", 7f)] [TestCase("8", 8f)] [TestCase("9", 9f)] [TestCase("0", 0f)] [TestCase("1.1", 1.1f)] [TestCase("2.9", 2.9f)] [TestCase("3.8", 3.8f)] [TestCase("4.7", 4.7f)] [TestCase("5.6", 5.6f)] [TestCase(".", 0f)] public void ParseSetValue_Test_AreEqual(string valueToParse, float expectedValue) { //arrange -done in setup //act number.ParseSetValue(valueToParse); //assert Assert.AreEqual(expectedValue, number.value); } [TestCase("$#.")] [TestCase("$#.432423")] [TestCase("+_/")] [TestCase("'~/?")] [TestCase("3.3.3")] [TestCase("")] public void ParseSetValue_Test_ThrowsException(string valueToParse) { Assert.Throws<Exception>(() => number.ParseSetValue(valueToParse)); } [TearDown] public void TearDown() { number = null; } } }
Is this the ultimate pro code? Nope. If you follow the steps we have been talking about, your code will be a cut above most coders in my experience. If you are a bit fuzzy about how to do something. Get it working then refactor to a more elegant design.
If I commissioned a four function calculator and this was delivered I would be pretty happy. That being said the joy and curse of the profession is there is always a better design or more improvements.
When I say this is the final cleanup of this Noob to Pro series, understand that there is final and then there is final.
Just off the top of my head, we could swap out the constants for enumerated types. We could decide we want our business logic to handle all invalid number cases instead of just throwing a custom exception. That is a completely valid design decision and the 100% coverage crowd would probably demand it since GUI validation is technically logic. We could improve our double testing situation in our unit tests with mocks. I could spend another 2 weeks worth of posts hacking on this and improving it further. That’s part of the lesson.
The Mona Lisa was Leonardo Di Vinci’s best known work. The very symbol of Western Art. It is also in the form it is in because he died. It was an uncompleted work. A commission never delivered. A cautionary tale for every Noob and Pro alike when they contemplate when to do the final cleanup on their projects.
While it is great in theory to produce something perfect, perfectionism leads to never completed code. Sometimes you just have to say enough and move on.
Who knows maybe we will do a round 2 on this little calculator and make it really sing or die of old age trying.
If you want to experiment with it that is great. In my experience there has been plenty of professional code that was not half this clean and organized so if you make it here. I would consider you a professional.
I hope you enjoyed this final cleanup post as well as the rest of the Noob to Pro series.