Reputation: 832
I am trying to make a form using C# that is similar to a calculator.
I have to code a method named IsOperator that checks that the text box that’s passed to it contains a value of +, -, *, or /.
For some reason it is not validating correctly.
I've tried changing the || to && and reversing the returns to false and true but nothing works.
When I try putting other things in the operator text box that are not + / - * the results turn into 0. Nothing ends up validating.
Code:
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;
namespace SimpleCalculator
{
public partial class Form1 : Form
{
public Form1()
{
InitializeComponent();
}
private void btnCalc_Click(object sender, EventArgs e)
{
try
{
if (IsValidData())
{
MessageBox.Show("Error");
}
else
{
decimal operand1 = Convert.ToDecimal(txtOperand1.Text);
string operator1 = txtOperator.Text;
decimal operand2 = Convert.ToDecimal(txtOperand2.Text);
decimal result = 0;
if (operator1 == "+")
result = operand1 + operand2;
else if (operator1 == "-")
result = operand1 - operand2;
else if (operator1 == "*")
result = operand1 * operand2;
else if (operator1 == "/")
result = operand1 / operand2;
result = Math.Round(result, 4);
txtResult.Text = result.ToString();
txtOperand1.Focus();
}
}
catch (Exception ex)
{
MessageBox.Show(ex.Message + "\n\n" +
ex.GetType().ToString() + "\n" +
ex.StackTrace, "Exception");
}
}
public bool IsValidData()
{
return
//validate the operand1 text box
IsPresent(txtOperand1, "Operand 1") &&
IsDecimal(txtOperand1, "Operand 1") &&
IsWithinRange(txtOperand1, "Operand 1", 0, 1000000) &&
//validates the operator text box
IsPresent(txtOperator, "Operator") &&
IsOperator(txtOperator, "Operator") &&
//validates the operand 2 text box
IsPresent(txtOperand2, "Operand 2") &&
IsDecimal(txtOperand2, "Operand 2") &&
IsWithinRange(txtOperand2, "Operand 2", 0, 1000000);
}
private void btnExit_Click(object sender, System.EventArgs e)
{
this.Close();
}
//is present
public bool IsPresent(TextBox textBox, string name)
{
if (textBox.Text == "")
{
MessageBox.Show(name + " is a required field.", "Entry Error");
textBox.Focus();
}
return false;
}
//is decimal
public bool IsDecimal(TextBox textBox, string name)
{
decimal number = 0m;
if (Decimal.TryParse(textBox.Text, out number))
{
return true;
}
else
{
MessageBox.Show(name + " must be a decimal value.", "Entry Error");
textBox.Focus();
return false;
}
}
//is within range
public bool IsWithinRange(TextBox textBox, string name, decimal min, decimal max)
{
decimal number = Convert.ToDecimal(textBox.Text);
if (number < min || number > max)
{
MessageBox.Show(name + "must be between" + min.ToString()
+ "and" + max.ToString() + ".", "Entry Error");
textBox.Focus();
return false;
}
return true;
}
//is a valid operator
public bool IsOperator(TextBox textBox, string name)
{
string operator1 = "";
operator1 = Convert.ToString(textBox.Text);
if (operator1 == "+" && operator1 == "-" && operator1 == "/" && operator1 == "*")
{
MessageBox.Show("Please enter a valid operator in the operator text box.", "Entry Error");
return false;
}
return true;
}
private void txtOperand1_TextChanged(object sender, EventArgs e)
{
this.txtResult.Text = "";
}
private void txtOperator_TextChanged(object sender, EventArgs e)
{
this.txtResult.Text = "";
}
private void txtOperand2_TextChanged(object sender, EventArgs e)
{
this.txtResult.Text = "";
}
}
}
Upvotes: 1
Views: 813
Reputation: 1126
Fist of all you should not use someString == ""
instead use string.IsNullOrEmpty(someString)
or string.IsEmptyOrWhitespace(someString)
. 1
Then IsPresent
always returns false
. You may change that method to
public bool IsPresent(TextBox textBox, string name)
{
if (!string.IsNullOrEmpty(textBox.Text))
{
return true;
}
MessageBox.Show(name + " is a required field.", "Entry Error");
textBox.Focus();
return false;
}
In the EventHandler your forgot a !
before IsValidData()
. You you're showing an Error, when the data is valid and try to perform the calculation, when your data is faulty.
Your IsOperator
Method contains in fact a logical issue. You want to return true, if the operator is any of the following chars +, -, *, \
. So its easier to flip the if
logic to something like this using LINQ
//is a valid operator
public bool IsOperator(TextBox textBox, string name)
{
string operator = textBox.Text;
if (new[] {"+", "-", "*", "/"}.Contains(operator))
{
return true;
}
MessageBox.Show("Please enter a valid operator in the operator text box.", "Entry Error");
return false;
}
I think your code also could benefit from using Exceptions instead of showing MessageBoxes as soon an error occurs.
Upvotes: 1