C++

Forum

 
Stack Help!

Edgy (8)
When I test it the stack seems to be filling up correctly, but the answer always comes out as zero. Everthing compiles fine so it is some logic error I have.

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
71
72
73
74
 
#include <iostream>
#include <string.h>
#include <stack>
#include <sstream>
#include <stdlib.h>
#include "IntStack.h"

using namespace std;

int main()
{
	        cout << "Welcome to my postfix notation calculator. \n";
                int i, num, result, topNum, secondNum, catchVar;
		char op, ch;
		string str, str1;

		IntStack stack;

		// get the line as a string
		getline(cin, str);
		istringstream is(str);

		// read code , adding it to stack and preforming operations when an operator is reached
		for (; is >> str1;){

			if (str1 == "+"){

				topNum = atoi(str1.c_str());
				stack.pop(catchVar);
				secondNum = atoi(str1.c_str());
				stack.pop(catchVar);


				stack.push(topNum + secondNum);

			}
			else if (str1 == "-"){

				topNum = atoi(str1.c_str());
				stack.pop(catchVar);
				secondNum = atoi(str1.c_str());
				stack.pop(catchVar);
				stack.push(secondNum - topNum);
			}
			else if (str1 == "*"){

				topNum = atoi(str1.c_str());
				stack.pop(catchVar);
				secondNum = atoi(str1.c_str());
				stack.pop(catchVar);
				stack.push(topNum * secondNum);
			}
			else if (str1 == "/"){

				topNum = atoi(str1.c_str());
				stack.pop(catchVar);
				secondNum = atoi(str1.c_str());
				stack.pop(catchVar);
				stack.push(secondNum / topNum);
			}
			// if there was no operator push the number onto the stack
			else{

				stack.push(strtof(str1.c_str(), NULL));
				cout << str1 << '\n';
			}
		}

		stack.pop(catchVar);
		cout << "The answer is:" << catchVar;
		}
	return 0;
	
}


IntStack.cpp
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
71
72
73
74
75
76
77
78
79
80
 
#include <iostream>
#include "IntStack.h"

using namespace std;


// destructor
IntStack::~IntStack()
{
	StackNode *nodePtr = nullptr, *nextNode = nullptr;

	//point nodePtr at top
	nodePtr = top;

	while (nodePtr != nullptr)
	{
		nextNode = nodePtr->next;
		delete nodePtr;
		nodePtr = nextNode;
	}
}


// function to push stack
void IntStack::push(int num)
{
	//point to new node
	StackNode *newNode = nullptr;

	//Make new node and store num
	newNode = new StackNode;
	newNode->value = num;

	//put on top if list is not empty, if emptry create it as top
	if (isEmpty())
	{
		top = newNode;
		newNode->next = nullptr;
	}
	else
	{
		newNode->next = top;
		top = newNode;
	}
}

// function to pop number from the top of the stack, will provide warning if the stack is empty
void IntStack::pop(int &num)
{
	StackNode *temp = nullptr; // create temp ptr

	//make sure not empty
	if (isEmpty())
	{
		cout << "The stack is empty. \n";
	}
	else // pop top num from stack
	{
		num = top->value;
		temp = top->next;
		delete top;
		top = temp;
	}
}

//check if the stack is empty
bool IntStack::isEmpty()
{
	bool status;

	if (!top)
	{
		status = true;
	}
	else
	{
		status = false;
	}
	return status;
}


IntStack.h
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
 
#ifndef INTSTACK_H_
#define INTSTACK_H_

#include <iostream>

class IntStack
{
private:
	struct StackNode
	{
		int value;
		StackNode *next;
	};

	StackNode *top;

public:
	// constructor
	IntStack()
	{
		top = nullptr; 
	}
	// destructor
	~IntStack();

	// stack operations
	void push(int);
	void pop(int &);
	bool isEmpty();

};


#endif /* INTSTACK_H_ */
Last edited on
TheIdeasMan (6798)
Everthing compiles fine so it is some logic error I have.


Consider using a debugger. There should be a GUI one in your IDE. If not command line versions exist.

Good Luck!!
Thomas1965 (4571)
Actually I had a compile error in line 71: 
1
2
3
 
cout << "The answer is:" << catchVar;
} // doesn't belong there
return 0;

When I removed the closing brace it run.

Welcome to my postfix notation calculator.
2+3*2-2
2+3*2-2
The answer is:2

For debugging you also could use this TRACE macro.

1
2
3
4
5
 
#ifdef _DEBUG
  #define TRACE(...) { printf(__VA_ARGS__); }
#else
  #define TRACE(...)
#endif
Last edited on
Edgy (8)
One thing that has really been lacking in my education is the debugging utilities. After studying those I should be able to solve these issues myself.

Its something i will talk with my professor about for the accelerated course we are doing.
Thomas1965 (4571)
One way of debugging is using Trace statements like this:
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
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
 
#include <iostream>
#include <string.h>
#include <stack>
#include <sstream>
#include <stdlib.h>
#include "Stack.h"

using namespace std;

#define _DEBUG

#ifdef _DEBUG
  #define TRACE(...) { printf(__VA_ARGS__); }
#else
  #define TRACE(...)
#endif

int main ()
{
  cout << "Welcome to my postfix notation calculator. \n";
  int i, num, result, topNum, secondNum, catchVar;
  char op, ch;
  string str, str1;

  IntStack stack;

  getline (cin, str);
  istringstream is (str);

  // read code , adding it to stack and preforming operations when an operator is reached
  for (; is >> str1;)
  {
    TRACE("Reading %s\n", str1.c_str());
    if (str1 == "+")
    {

      topNum = atoi (str1.c_str ());
      TRACE("topNum = %d\n", topNum);
      stack.pop (catchVar);
      TRACE("Popping catchVar=%d\n",catchVar);
      secondNum = atoi (str1.c_str ());
      TRACE("secondNum = %d\n", secondNum);
      stack.pop (catchVar);
      TRACE("Popping catchVar=%d\n",catchVar);
      stack.push (topNum + secondNum);
      TRACE("Pushed %d to stack\n", topNum + secondNum);

    }
    else if (str1 == "-")
    {

      topNum = atoi (str1.c_str ());
      stack.pop (catchVar);
      secondNum = atoi (str1.c_str ());
      stack.pop (catchVar);
      stack.push (secondNum - topNum);
    }
    else if (str1 == "*")
    {

      topNum = atoi (str1.c_str ());
      stack.pop (catchVar);
      secondNum = atoi (str1.c_str ());
      stack.pop (catchVar);
      stack.push (topNum * secondNum);
    }
    else if (str1 == "/")
    {

      topNum = atoi (str1.c_str ());
      stack.pop (catchVar);
      secondNum = atoi (str1.c_str ());
      stack.pop (catchVar);
      stack.push (secondNum / topNum);
    }
    // if there was no operator push the number onto the stack
    else
    {
      stack.push (strtof (str1.c_str (), NULL));
      TRACE("Pushed %d to stack\n", strtof (str1.c_str (), NULL));
      cout << str1 << '\n';
    }
  }

  stack.pop (catchVar);
  cout << "The answer is:" << catchVar;
  
  system ("pause");
return 0;

}


Output:
Welcome to my postfix notation calculator.
1 2 +
Reading 1
Pushed 0 to stack
1
Reading 2
Pushed 0 to stack
2
Reading +
topNum = 0
Popping catchVar=2
secondNum = 0
Popping catchVar=1
Pushed 0 to stack
The answer is:0

Maybe this helps you a bit.

TheIdeasMan (6798)
From the compile error that Thomas1965 had, the lesson there is to set the warning / error levels to their highest possible.

With g++ , one should at least use -Wall -Wextra -pedantic. IMO, the situation with VS is a little more awkward: the default level (-w3) is too low, the next (-w4) which is the highest finds lots of errors in the standard include files, so I guess one has to resort to setting individual warnings.

The clang++ compiler is probably (IMHO) one of the best freely available compilers: it has nice messages, and seems to be a frontrunner in terms of compliance with latest standards.

Also, your editor should be able to match pairs of braces or brackets automatically. If it doesn't or can't be set up to do so, then revert to old school method of typing both opening and closing braces, then go back and fill in what should be inside them. That way you will never be out of whack. I did that for about 20+ years, then had to unlearn that when I had an editor with that capability.

The program is a RPN stack, isn't it? So 2+3*2-2 should be 2 3 2 * + 2 - which is 6 ? If I am not mistaken. The 2 was produced with the first input because that was the last value placed on the stack.

Learning to use a GUI debugger should be really easy, basically say which variables you want to keep track of on a watch-list, set a beak point somewhere before where you think there is a problem, then step through 1 line at a time.

With a command line debugger such as gdb, there is a tutorial article in the articles section top left of this page, and a man page should be easily found on the web.
TheIdeasMan (6798)
24
25
26
27
28
29
30
31
32
33
34
35
36
 
		for (; is >> str1;){

			if (str1 == "+"){

				topNum = atoi(str1.c_str());
				stack.pop(catchVar);
				secondNum = atoi(str1.c_str());
				stack.pop(catchVar);


				stack.push(topNum + secondNum);

			}


I am a bit confused by what you are doing here: Shouldn't line 24 be a while loop? And Line 28 sets topNum to 43 (the ASCII value) because str1 is "+"

You don't use catchVar in the addition expression.

With division, you don't seem to check for division by zero. And integer division won't work. Use static_cast<double>(TopNum) to cast one of the numbers to double.

Hope this helps a bit :+)
Thomas1965 (4571)
@TheIdeasMan

IMO, the situation with VS is a little more awkward: the default level (-w3) is too low, the next (-w4) which is the highest finds lots of errors in the standard include files, so I guess one has to resort to setting individual warnings.


I am reading Debugging Windows Programs by Everett N. McKay and Mike Woodring and they recommend to use always the /W4 warning level.

To prevent problems compiling some libraries they recommend the following solution:


#prgma warning (push,3) // temporarily revert to /W3
#include "difficultLib.h"
#pragma warning(pop) // revert back to previous settings


Make sense to me, we should be concerned about compiling our own code without warnings.
TheIdeasMan (6798)
Thomas1965 wrote:
Make sense to me, we should be concerned about compiling our own code without warnings.


Yes, I agree.

Although it strikes me as a little odd to have to put (non standard? ) pragmas into code, in order for it to compile with helpful warnings / errors or for it to not display heaps of spurious warnings.

Last time I dabbled with VS, I didn't look in detail as to what the warnings were for, for STL headers, but I remember there being pages of them. Was trying to read about it, but can't connect to MSDN website.

I guess I prefer to use clang on Linux and not have to worry about any of this :+)
Topic archived. No new replies allowed.