please help to make calculator using stack to evaluate postfix

#include <iostream>
#include <conio.h>
#include <string.h>

using namespace std;

struct node{
int data;
node *next;
}*p = NULL, *top = NULL, *save = NULL, *temp;

class stack{
public:
void push(int x){
p = new node;
p->data = x;
p->next = NULL;
if (top == NULL){
top = p;
}else{
save = top;
top = p;
p->next = save;
}
}

char pop(){
if (top == NULL){
cout<<"Empty!!";
}else{
temp = top;
top = top->next;
return(temp->data);
delete temp;
}
}
};


int main(){
stack obj;
char x[30];
int a, b;
cout<<"enter the balanced expression\n";
cin>>x;
for (int i = 0; i < strlen(x); i++){
if (x[i] >= 48 && x[i] <= 57)
obj.push(x[i]-'0');
else if (x[i] >= 42 && x[i] <= 47){
a=obj.pop();
b=obj.pop();
switch(x[i]){
case '+':
obj.push(a+b);
break;
case '-':
obj.push(a-b);
break;
case '*':
obj.push(a*b);
break;
case '/':
obj.push(a/b);
break;
}
}
}
cout<<"Answer: "<<obj.pop()<<endl;
getch();
}
Last edited on
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
#include <iostream>
#include <conio.h>
#include <string.h>

using namespace std;

struct node{
    int data;
    node *next;
}*p = NULL, *top = NULL, *save = NULL, *temp;

class stack{
	public:
	void push(int x){
	    p = new node;
	    p->data = x;
	    p->next = NULL;
	    if (top == NULL){
	        top = p;
	    }else{
	        save = top; 
	        top = p;
	        p->next = save;
	    }
	}
	
	char pop(){
	    if (top == NULL){
	        cout<<"Empty!!";
	    }else{
	        temp = top;
	        top = top->next;
	        return(temp->data);
	        delete temp;
	    }
	}
};
	

int main(){
    stack obj;
	char x[30];
    int a, b;
    cout<<"enter the balanced expression\n";
    cin>>x;
    for (int i = 0; i < strlen(x); i++){
        if (x[i] >= 48 && x[i] <= 57)
            obj.push(x[i]-'0');
        else if (x[i] >= 42 && x[i] <= 47){
            a=obj.pop();
            b=obj.pop();
            switch(x[i]){
            case '+':
                obj.push(a+b);
                break;
            case '-':
                obj.push(a-b);
                break;
            case '*':
                obj.push(a*b);
                break;
            case '/':      
                obj.push(a/b);
                break;
            }
        }
    }
    cout<<"Answer:	"<<obj.pop()<<endl;
    getch();
}
Last edited on
please help me
There are a few things about the design which could be improved - I'd get rid of the global variables p, top, save, temp

top should be a private data member of the class stack. All the others are temporary variables which can be declared locally within the scope where they are required to be used.

The use of ASCII codes such as 48, 57 would be more clearly understood as the characters '0' and '9' while this line
 
    if (x[i] >= 42 && x[i] <= 47)
is a poor compromise, since it accepts all of these characters: "*+,-./" but it should only allow "*+-/".
I'd suggest changing that to
 
    if ( strchr("+-*/", x[i]) )

see http://www.cplusplus.com/reference/cstring/strchr/

As for the more serious errors in the code, the pop() function is flawed in more than one way.
27
28
29
30
31
32
33
34
35
36
	char pop(){
	    if (top == NULL){
	        cout<<"Empty!!";
	    }else{
	        temp = top;
	        top = top->next;
	        return(temp->data);
	        delete temp;
	    }
	}

1. The node contains data of type int but the function is returning a char.
2. Control may reach the end of the function at line 36 without returning any value. It must always return something.
3. The return statement at line 33 exits from the function. That means the delete at line 34 can never be executed. That gives a memory leak - memory allocated which can never be released.

Something like this could work:
27
28
29
30
31
32
33
34
35
36
37
38
39
    int pop()    // return an integer
    {
        if (top)
        {
            node * temp = top;
            top = top->next;
            int data = temp->data;
            delete temp;
            return data;
        }
        cout << "Empty!!";
        return 0;   // must return something!
    }




You may find the following thread on a related topic of interest.
http://www.cplusplus.com/forum/general/203063/
Last edited on
Topic archived. No new replies allowed.