Code works - need advice on logic

[BagoNone]

Heya all. New to programming, and new to the forum. My latest homework assignment is complete and working, but I know the logic can probably be made more efficient/straightforward/something.

I welcome any advice you have on what I did that could be done better/faster/easier in the following program. It's the basic "take a string and ensure that the mathematical expressions are correct" problem.

3 files. A main .cpp, and a linked-list stack to hold the character being processed.

Thanks in advance!

***Class Header file***
#pragma once
#include <iostream>

using namespace std;

struct node
{
    char ch;
    node *link;
};


class CharLinkedListStack
{
private:
    node *stackTop;

public:
    CharLinkedListStack();
    ~CharLinkedListStack();

    bool isEmpty() {return stackTop == NULL;}
    bool isFull() {return false;}

    void push(char);
    void pop();
    char top();

};


***Class .cpp file***
#pragma once
#include "CharLinkedListStack.h"

CharLinkedListStack::CharLinkedListStack()
{
    stackTop = NULL;
} // end default (empty) constructor


CharLinkedListStack::~CharLinkedListStack()
{
    while (! isEmpty())
        pop();
} // end deconstructor

void CharLinkedListStack::push(char chIn)
{
    node *newNode = new node();
    newNode->ch = chIn;
    newNode->link = stackTop;

    stackTop = newNode;
} // end push function


void CharLinkedListStack::pop()
{
    if (! isEmpty())
    {
        node *temp = stackTop;
        stackTop = stackTop->link;
        delete temp;
    }

    else
        cerr << "Error! Cannot pop from an empty stack!" << endl;
} // end pop function


char CharLinkedListStack::top()
{
    if (! isEmpty())
        return stackTop->ch;
    else
        cerr << "Error! Cannot read from an empty stack!" << endl;
    return -1;
} // end top function


*** Main .cpp driver file***
#include <iostream>
#include <string>
#include "CharLinkedListStack.h"

using namespace std;

int main()
{
	// declare LinkedList
	CharLinkedListStack mathList;

    // declare variables
    char ch;				// variable to hold current character being checked
    int counter = 0;		// variable to hold counter of location in string
    string input;			// variable to hold user input
    bool isValid = true;	// variable to trigger valid expression

    // prompt user to enter a mathematical expression
    cout << "Enter an arithmetic expression: ";
    getline(cin, input);

    // display the string
    cout << "You entered: " << input << endl;
	cout << "Length of string is: " << input.length() << endl;
	ch = input[0];
	
	while (counter < input.length() && (isValid))			// while we have not reached the end of the string
	{
		ch = input[counter];
		cout << "ch == " << ch << endl;

		switch (ch)
		{
			case '(':
			case '{':
			case '[':
			{
				mathList.push(ch);
				cout << mathList.top() << " is the top of the stack" << endl;
				counter++;
				//ch = input[counter];
				continue;
			}
			
			case ')':
			{
				if (! mathList.isEmpty())		// if the list is not empty
				{
					if (mathList.top() != '(')	// if the close paren doesn't have a matching open paren
					{
						// cout << "That expression does not contain matching group symbols!" << endl;
						isValid = false;
						break;
					} // end inner if

					// found a matching pair, so pop the top of the stack off
					else
					{
						mathList.pop();
						isValid = true;
						counter++;
						continue;
					} // end inner else
				} // end if 

				else // list is empty
				{
					isValid = false;	// List is empty, so close paren is also wrong
					break;
				}
			} // end ')' case

			case '}':
			{
				if (! mathList.isEmpty())		// if list is not empty
				{
					if (mathList.top() != '{')	// if close curly doesn't have matching open curly
					{
						// cout << "That expression does not contain matching group symbols!" << endl;
						isValid = false;
						break;
					} // end inner if

					else
					{
						mathList.pop();
						isValid = true;
						counter++;
						continue;
					} // end inner else

				} // end outer if

				else // list is empty
				{
					isValid = false;	// List is empty, so close curly is also wrong
					break;
				}
			} // end '}' case

			case ']':
			{
				if (! mathList.isEmpty())		// if list is not empty
				{
					if (mathList.top() != '[')	// if close bracket doesn't have matching open bracket
					{
						// cout << "That expression does not contain matching group symbols!" << endl;
						isValid = false;
						break;
					} // end inner if

					else
					{
						mathList.pop();
						isValid = true;
						counter++;
						continue;
					} // end inner else
				} // end outer if

				else // list is empty
				{
					isValid = false;	// List is empty, so close bracket is also wrong
					break;
				}
			} // end ']' case

			default:
			{
				// character is not one of the important ones
				if (counter < input.length())
				{
					cout << "counter is: " << counter << endl;
					counter++;
					continue;
				}
			} // end default case
					
		} // end switch

	} // end while

	if (isValid && mathList.isEmpty())	// String is valid, and stack is empty
		cout << "\nThat expression contains matching group symbols." << endl << endl;
	else 
		cout << "\nThat expression does not contain matching group symbols!" << endl << endl;

system("pause");
return 0;
}

[MiiNiPaa]

Your stack implementation is fine (However I would remove isFull() function, renamed isEmpty() to empty() and and made Node nested private struct). Also your main code can be made way shorter and clearer.

(I have used std::stack in this demonstration)

#include<iostream>
#include<string>
#include<stack>


bool balanced(std::string input)
{
    static const std::string op_braces = "({[";
    static const std::string cl_braces = ")}]";

    std::stack<char> braces;
    for(auto c: input) {
        auto pos = std::string::npos;
        if (op_braces.find(c) != std::string::npos) {
            braces.push(c);
        } else if ( (pos = cl_braces.find(c)) != std::string::npos ) {
            if (braces.empty())
                return false;
            if (braces.top() == op_braces[pos])
                braces.pop();
            else
                return false;
        }
    }
    if (!braces.empty())
        return false;
    return true;
}


int main ()
{
    std::string input;
    std::getline(std::cin, input);
    std::cout << "That expression " << (balanced(input) ? "contains" :
                                                          "does not contain") <<
                 " matching group symbols\n";

}

Edit: forgot to include empty check

[BagoNone]

Wow - thanks for the detailed example! For some reason, our Instructor wants us to include an isEmpty() function, although it seems to be completely useless for a Linked-List. That's why that one is in there. I'll also look into making the Node struct a private member instead. Thanks again!

And your code is definitely much more concise than mine. I'll have to research into some of the <stack> class usage, since we haven't worked with that one yet. But from appearances, it seems like like "auto c:" is picking out each character in the string, and if it finds a closing brace, looking at the top of the stack for a matching opening brace. Then it returns the corresponding boolean value. Is that correct?

Thanks again for the advice - greatly appreciated!

[MiiNiPaa]

You can simply change std::stack<char> to your CharLinkedListStack and it will work (you will have to change empty() to isEmpty() too)

it seems like like "auto c:" is picking out each character in the string

Yes. You can write for(char c: input) if you want clarity.

if it finds a closing brace, looking at the top of the stack for a matching opening brace.

Basically, yes. It stop analisys and returns false if it suddenly finds closing brase not matching last opening, if it finds a closing brace when there is no matching opening and if after iterating over string there still is unmatched opening braces in stack.
If everything is fine, it returns true

[BagoNone]

Makes sense. I'll still review the <stack> class, since it seems like a lot of this work is already included in the STL, (but we're not allowed to use the STL for these exercises). And more knowledge is never a bad thing!

Again, greatly appreciate the explanation and help.