Code to check mismatch symbols doesn't work

[anderfernandes]

All,

I created code to check if there are parentheses (), Braces {} and Brackets [] that are not closed. Here is what I came up with:

#include <iomanip>
#include <iostream>
#include <string>
#include <fstream>
#include <stack>
const string LEFTGROUP="{[(";
const string RIGHTGROUP="}])";

fin.open("D:\\Advanced Cpp\\Visual Studio\\MatchingGroupSymbols\\Debug\\ProgramA.cpp");
 			if(fin.fail())
			{
				cout<<"Program failed to open file."<<endl;
				return 0;
			}
			while(!fin.eof())
			{
				fin>>ch;

				//push left group symbols onto stack
				if(ch==LEFTGROUP[0])
					stk.push(ch);
				else if(ch==LEFTGROUP[1])
					stk.push(ch);
				else if(ch==LEFTGROUP[2])
					stk.push(ch);
		
			}
			fin.close();
			fin.open("D:\\Advanced Cpp\\Visual Studio\\MatchingGroupSymbols\\Debug\\ProgramA.cpp");
			while(!fin.eof())
			{
				fin>>ch;

				if(stk.top()==LEFTGROUP[0])
				{
					if(ch==RIGHTGROUP[0])
						stk.pop();
				}
				else if(stk.top()==LEFTGROUP[1])
				{
					if(ch==RIGHTGROUP[1])
						stk.pop();
				}
				else if(stk.top()==LEFTGROUP[2])
				{
					if(ch==RIGHTGROUP[2])
						stk.pop();
				}
			}
			fin.close();

			if(!stk.empty())
				cout<<"File has mismatched symbols."<<endl;
			else if(stk.empty())
				cout<<"File does not have mismatched symbols."<<endl;

			cout << "\n\nProgram complete." <<  endl;

This is inside a case statement. As you can see, it reads a .cpp file and try to find mismatch symbols. The example below doesn't have any mismatched symbols but it still says that it does. The file is open without problems.

#include <iostream>
using namespace std;

int main()
{
	cout << "The result of the expression {[32 * (12 + 5) - 2]} + 17 is "
		 << ((32*(12+5)-2)+17) << endl;
	return 0;

Why is it doing that? Is my logic wrong?

[jim80y]

Apart from the test code missing a closing } (which I'm assuming is a posting mistake), it looks like it should work.

Can you print out the contents of the stack at the end, to see what char(s) it thinks is/are mismatched?

Jim

[anderfernandes]

Where exactly do you see a missing closing } ? As you can see, this line has a pair of braces:

cout << "The result of the expression {[32 * (12 + 5) - 2]} + 17 is "
		 << ((32*(12+5)-2)+17) << endl;

If it is not here, where else is it?

[jim80y]

The closing brace of main() ?

[Lowest0ne]

Line 9 should be a closing brace. You also have issues with closing braces that do not have openings: [)] would not report an error.

Reading the file twice isn't helping you. The stack is basically a collection of expected closing characters:

-openers = "({["
-closer = ")}]"

-while read ch
  -if ch is in openers
    - stack.push the corresponding closer
  -else if ch is in closers
    - if ch != stack.top
      - mismatch
-if stack not empty
  - mismatch

If this is your approach, then you can know exactly where in the file you are:

if (ch != stack.top())
{
  std::cout << "Mismatch: " << ch << "-" << stack.top << " "<< input.tellg() <<  '\n';
}

[anderfernandes]

Sorry, I do close the braces in main. How do I print the contents of stack?

[jim80y]

while (!stack.empty())
{
  cout << stack.top() << endl;
  stack.pop();
}

Jim

[anderfernandes]

This is the output when I put the code to print the stack:

Which program you would like to check for pairs of grouping symbols?

1 - ProgramA.cpp
2 - ProgramB.cpp
3 - ProgramC.cpp

Please select an option: 1
File has mismatched symbols.


Program complete.
[
{
{
(
Press any key to continue . . .

Looks like it's finding only the opening symbols...

[jim80y]

Yeah, you're pushing the search characters (the left-hand ones) onto a stack, which means you'll be popping them in reverse order. But you're testing the closing characters in original order (when you read the file a second time).

That's not going to work.

Plus, you always look for the matching closing character for the char on the top of the stack. If you happen to come across a different closing char while looking for that specific one, you'll skip it and totally ignore it, leaving it in the stack at the end.

Jim

[anderfernandes]

I understand. I'm gonna try creating an array with the left symbols, right symbols, have it run a loop to find those symbols and, if there is an odd number of braces for example, the program will report a mistake. Let me know if anybody has a better idea. Thanks.

[jim80y]

@LowestOne's algorithm would be better

[anderfernandes]

I tried to convert @LowestOne's algorithm but I don't think I'm doing it right. Here is his algorithm:

-openers = "({["
-closer = ")}]"

-while read ch
  -if ch is in openers
    - stack.push the corresponding closer
  -else if ch is in closers
    - if ch != stack.top
      - mismatch
-if stack not empty
  - mismatch

Here is what I came up with:

const string LEFTGROUP="{[(";
const string RIGHTGROUP="}])";
		

if(ch==LEFTGROUP[0])
					stk.push(ch);
				else if(ch==LEFTGROUP[1])
					stk.push(ch);
				else if(ch==LEFTGROUP[2])
					stk.push(ch);
				
				else if(ch==RIGHTGROUP[0])
					if (ch != stk.top())
						cout<<"File has mismatched symbols."<<endl;

What am I missing?

[jim80y]

You need to be pushing the corresponding RIGHTGROUP char, not ch itself.
Plus the algorithm is missing a pop off the stack for each match found.
You also need to re-order your loop slightly to re-check eof after you've tried reading the next char, not before.

	fin >> ch;
	while(!fin.eof())
	{
		if (ch == LEFTGROUP[0])
			stk.push(RIGHTGROUP[0]);
		else if (ch == LEFTGROUP[1])
			stk.push(RIGHTGROUP[1]);
		else if (ch == LEFTGROUP[2])
			stk.push(RIGHTGROUP[2]);
		else if (ch == RIGHTGROUP[0] || ch == RIGHTGROUP[1] || ch == RIGHTGROUP[2])
		{
			if (stk.empty() || ch != stk.top())
				cout << "Mismatch at " << fin.tellg() << endl;
			else
				stk.pop();
		}
		
		fin >> ch;
	}

[anderfernandes]

I gave up on stacks because I'm not confident in using then and used a different approach.

fin>>ch;
			while(!fin.eof())
			{				
				if(ch==LEFTGROUP[0])
					openers++;
				else if(ch==LEFTGROUP[1])
					openers++;
				else if(ch==LEFTGROUP[2])
					openers++;

				if(ch==RIGHTGROUP[0])
					closers++;
				if(ch==RIGHTGROUP[1])
					closers++;
				else if(ch==RIGHTGROUP[2])
					closers++;

				fin>>ch;
		
			}

Your line before the while loop helped me figure out a problem that I was having. The number of closing symbols was always greater then what they were supposed to be.

Basically it stores the numbers of opening and closing symbols in two different variables. If the sum of opening and closing symbols is even, then it matches (e.g. 2 opening and two closing parenthesis is a match), but if it is odd (e.g. 2 opening braces and 3 closing braces) then it doesn't match. It is not an awesome approach but it hasn't failed so far. Thanks a lot, guys!

PS: I tried your approach too @jim80y and it works flawesly.

PS2: Now I just need to figure out how to put that in a function because this portion of code will be repeated three times.

[jim80y]

If you're doing it that way you'll lose the ability to determine which character is mismatched. If you're never going to want that information, that's OK, but I'd think you may do if you ever extend this program.

Assuming you're going with this method, though, you'd be better of using the string::find() function to determine whether ch is in either of the left/right strings...

if (LEFTGROUP.find(ch) != string::npos)
  openers++;
else if (RIGHTGROUP.find(ch) != string::npos)
  closers++;

If you don't mind losing the separate counts, even the following will do the job:

int difference = 0;
...
while()
{
...
  if (LEFTGROUP.find(ch) != string::npos)
    difference++;
  else if (RIGHTGROUP.find(ch) != string::npos)
    difference--;
}

if (different != 0)
  cout << "Mismatch" << endl;

[anderfernandes]

Works perfectly. Thanks a lot!