user1472747
user1472747

Reputation: 567

Why isn't my PHP calculator working? (string functions..)

This calculator is supposed to take a typed input with spaces, (like "2 + 2") in the web page text box, and then echo the answer above it. Currently, the output is either "0", or it will seemingly clip a random number off of the input.

I'm 95% sure the problem is within the substr functions - they are not being assigned the correct values. This is my guess because of the output behavior. To demonstrate,

1 + 1 will = 0 2 + 2 will = 0 ... 9 + 9 will = 0 10 + 10 will = 1 20 + 20 will = 2

note: The $firstNumber, $secondNumber, and $operator initial declaration values starting on line 16 are arbitrary.

Here are substr( , ) examples from PHP manual:

echo substr('abcdef', 1);     // bcdef   <br>
echo substr('abcdef', 1, 3);  // bcd<br>
echo substr('abcdef', 0, 4);  // abcd<br>
echo substr('abcdef', 0, 8);  // abcdef<br>
echo substr('abcdef', -1, 1); // f<br>

.

//'if' is executed when submit is pressed
if (isset($_POST['test']))
{
    $input = $_POST['test'];
    $answer = calculate($input);
    echo $answer;
}

//does string processing and number calculation
function calculate($input)
{
    //declarations (random)
    $firstNumber = 20;
    $secondNumber = 30;
    $operator = "+";
    $answer = 7;

    //string processing. 
    for($i = 0; $i <= strlen($input); $i++)  
    {
        //if current position of the string scan is an operator,
        if( trim(substr($input, $i, $i + 1), " ") == "+" ||trim(substr($input, $i, $i + 1), " ") == "-"
          ||trim(substr($input, $i, $i + 1), " ") == "*" ||trim(substr($input, $i, $i + 1), " ") == "/")
        {
            //then 

            //$operator = current position TO current position + 1
            $operator = substr($input, $i, $i + 1);
            //trim $operator
            $operator = trim($operator, " ");

            //$firstNumber = 0 TO current position - 1
            $firstNumber = substr($input, 0, $i - 1);
            //trim $operator
            $firstNumber = trim($firstNumber, " ");

            //$secondNumber = current position + 1 TO end of string
            $secondNumber = substr($input, $i + 1, strlen($input));
            //trim $operator
            $secondNumber = trim($secondNumber, " ");
        }

    }

    //if operator is ... then do that operation. 
    //example: if "+", then add $firstNumber and $secondNumber

    if($operator == "+")
    {
        $answer = $firstNumber + $secondNumber;
    }
    else if($operator == "-")
    {
        $answer = $firstNumber - $secondNumber;
    }
    else if($operator == "*")
    {
        $answer = $firstNumber * $secondNumber;
    }
    else if($operator == "/")
    {
        $answer = $firstNumber / $secondNumber;
    }

    //return the calculated answer for echo
    return $answer;
}

?>

<form action="test.php" method="POST">

    <textarea name="test" rows="3" cols="30"></textarea>
    <input type="submit" value="calculate!">

</form>

Upvotes: 3

Views: 415

Answers (1)

Dancrumb
Dancrumb

Reputation: 27539

Your problem is your use of substr.

substr take a string, a start position and a length. It's the length that you are misusing. As you pan through the input string, you are increasing the length of the substring that you are looking for. Since operators have length of 1, you should be using

    //if current position of the string scan is an operator,
    if( trim(substr($input, $i, 1), " ") == "+" || trim(substr($input, $i, 1), " ") == "-"
      ||trim(substr($input, $i, 1), " ") == "*" || trim(substr($input, $i, 1), " ") == "/")
    {
         ....

You also need to update your code here:

//$operator = current position TO current position + 1
$operator = substr($input, $i, 1);
//trim $operator
$operator = trim($operator, " ");

You could simplify this a little by doing

  potentialOperator = trim(substr($input, $i, 1), " ")

and comparing that to your supported operators. That'll save you multiple unnecessary calls to trim and substr It'll also mean that you identify the operator once and once only.

You might also want to look at preg_split with PREG_SPLIT_DELIM_CAPTURE for parsing the input string, instead of scanning it char by char.

Upvotes: 4

Related Questions