Basic Calculator program help!!

[manish1991]

Hi guys.

I am new to C++ and am trying to learn the language.
I have had a go at designing and creating a basic calculator program however a bit stuck.
My program kind of works but when the user hits the = sign the calculation is always 0 for some reason.

The programs purpose is to perform a simple calculation ie. (5)*(5)=25.

note the brackets for each number.

also I can't seem to figure out how to add a store previous value operation and make the calculator loop itself after each calculation.

i cant understand where i am going wrong. maybe a second pair of eyes could help. lol

any help would be much appreciated.

here is my code so far:

// manishCalculator.cpp : Defines the entry point for the console application.

#include "stdafx.h"
#include <iostream>
#include <math.h>
#include <conio.h>
#include <string>
using namespace std;

const int MAX_LINE_LENGTH = 100;
const int MAX_NUMBERS = 10;
const int MAX_OPERATORS = 10;
const float PI = 3.1415;
int numIndex = 0;	// index into numbers array
int opIndex = 0;	// index into operators array

float processCalculation(float a, float b, char op){
		float result = 0;
		switch(op){
			case '-': result = a - b; break;
			case '+': result = a + b; break;
			case '/': result = a / b; break;
			case '*': result = a * b; break;
			case '^': result = a; for(int c = 1; c<b; c++){result = result * a;}; break;
			case '%': a = 0.01 * a; result = b * a; break;
		}
		return result;
}


//This is supposed to work out the format of the whole input string, split by brackets, then pass the "processCalculation" 
// method to perform each individual calculation. 
float processEquation(string input[]){
	//Declare variables to be params for processCalculation
	float a = 99999;
	float b = 99999;
	char c = 'E';
	for(int x=0;x<sizeof(input);x++){
		if(input[x] == "pi" || "Pi" || "PI"){
			input[x] = "3.1415926535";
		}if(input[x] == "(" ){
			int counter = 1;
			int end = sizeof(input)-1;

			//get the location of the closing ")"
			for(int y = x;y < sizeof(input); y++){
				if(input[y] == "("){
					counter++;
				}else if(input[y] == ")"){
					counter--;
					if(counter == 0){
						end = y;
						break;
					}
				}
			}
			string* subarray;
			subarray = new string[(end-x)-2];
			//make smaller array containing bracket contents
			for(int y=0;y<sizeof(subarray);y++){
				subarray[y] = input[(y+x)+1];
			}
			//recursively call processEquation with smaller array
			processEquation(subarray);
		}else{
			if(a != 99999){
				if(c != 'E'){
					if(b != 99999){
						a = processCalculation(a,b,c);
						b = 99999;
						c = 'E';
					}else{
						b = atoi(input[x].c_str());
					}
				}else{
					for(int z = 0;z<sizeof(input[x]);z++){
						if((input[x]).at(z) != ' '){
							c = input[x].at(z);
							break;
						}
					}
				}
			}else{
				a = atoi(input[x].c_str());
			}
		}
	}
	return a;
}


void main()
{
	char key;
	char inputStr[MAX_LINE_LENGTH];
	int index = 0;

	std::cout << ("Please enter your equation to be calculated... \n");

	// read user input until the '=' key is pressed
	do{
		key = _getch();
		// validate the key here if you want to
		// and print only if it's valid
		inputStr[index++] = key;
		std::cout << key;
	}while(key != '=' && index < MAX_LINE_LENGTH);
	
	// now parse the user input placing each token into a new position in an array
	// the following assumes that all numbers and operators are separated by spaces
	// e.g.    3.12 * -12.56 =
	string input[MAX_LINE_LENGTH];
	string temp = "";
	int count = 0;
	for(int x=0;x<index;x++){
		if(!inputStr[x] ==  ' '){
			temp += inputStr[x];
		}else{
			input[count] = temp;
			temp = "";
			count ++;
		}
	}
	float output = processEquation(input);

	//TODO add extra processing stuff here

	std::cout << "\n the result is: " <<  output << "\n";
	std::getchar();
}

[MiiNiPaa]

(input[x] == "pi" || "Pi" || "PI") will always evaluate to true.
You want (input[x] == "pi" || input[x] == "Pi" || input[x] == "PI") here.
Also it looks like (5) string will be passed to input array as "(5)" and not as "(", "5", ")" which probably what you want. That means if(input[x] == "(" ) will never be true

[Glandy]

to add to MiiNiPaas findings:

remember at compile-time the function "processEquation" has no idea about the size of the array that is going to be passed to it (well it could check what Max-Array-size "input" has but it doesn't ;) (wouldn't even really be benefiting us))
which leads to =>
sizeof(input)
will probably always evaluate to 4, no matter what you really put into input

Edit: In fancy words: "sizeof" is a "compile-time" operation.

[MiiNiPaa]

Oh, yes, but even if it did know about size of array it still won't work, sizeof return memory size in bytes, not amount of elements in container.

[Glandy]

Ya of course it doesn't make sense to use sizeof there at all. But that should be something he has to figure out himself! ;)
Just giving him a little hint, there is something not quite right :P

Btw. @manish1991: The key to be good at those kind of "algorithms" is to bang your head as long as it takes against a wall until it finally works. ^.^
The learning-process how to build the algo and fix all those little misstakes it the actual stuff to learn here.
Believe me once it works. You are happy for a day and then forget about it and go on to something cooler with the help of the experience solving the previous problem =)

Something general I'd like to drop here: Whyy always make a big mix between C and C++ stuff especially strings ^.^
Why reading it in an inconvenient char-Array and not directly into a great easy-to-use and dynamic string ;x
You can check afterwards if it's a valid term (it just feels right instead of typing in every character itself ;>)

[TheIdeasMan]

First, your code is a good effort for a beginner :+) , so well done!

Some minor things to add:

It is always int main() not void main()

Prefer to declare functions before main, then define them after main.

Use double rather than float. float has only 7 or 8 significant figures of precision - this is easily exceeded. double has 15 or 16.

You had a const value for PI , but didn't use it on line 40.

Every beginner seems to forget to check for divide by zero. Be careful with that - check if the absolute value of the number is less than some arbitrary precision 0.0001 say.

Always put a default case in a switch - use it to catch bad input or error conditions.

I am not a fan of multiple statements on one line.

Always initialise variables - this is one of the biggest source of errors. Do this at declaration, one per line plus a comment, especially if it is only 1 char long - say what it means and it's use and the expected valid range or values. Try to use meaningful names wherever you can.

There is an *= operator for multiplication. There are a lot of these assignment operators - look them up in the tutorial.

You don't need line 8 if you are going to qualify std namespace things as in std::cout. Line 8 is bad practice, qualifying things with std:: is good practice.

Hope this helps and good luck - look forward to seeing your revised code.