pyuntae
pyuntae

Reputation: 832

C# Simple Calculator Form not validating correct operators?

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

Answers (1)

Pretasoc
Pretasoc

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

Related Questions