patterncppModerate
Validating opening and closing bracket pairs
Viewed 0 times
pairsbracketvalidatingandopeningclosing
Problem
I have refactored one of my old homework assignments (mainly to utilize
It reads in a text file, character by character, and determines whether all of the opening and closing brackets (
If a mismatch is found, an error message will be displayed to specify the specific one, and the program will terminate. Otherwise, at the end, a message will be displayed to indicate that they all match.
Here are the possible error types:
-
missing opening bracket:
-
missing closing bracket:
-
opening bracket closed with the wrong closing bracket:
My questions:
I don't mind following an entirely different procedure if this one isn't very practical. If you have something more complicated in mind, please share it. I want to approach this the right way.
```
#include
#include
#include
#include
#include
type
std::stack and some C++11), and I'm still having trouble making it less repetitive.It reads in a text file, character by character, and determines whether all of the opening and closing brackets (
{}, (), and []) match properly. All other characters are ignored.If a mismatch is found, an error message will be displayed to specify the specific one, and the program will terminate. Otherwise, at the end, a message will be displayed to indicate that they all match.
Here are the possible error types:
-
missing opening bracket:
int main() { /*...*/ } ] // missing [-
missing closing bracket:
int main() { /*...*/ // missing }-
opening bracket closed with the wrong closing bracket:
int main() { /*...*/ ] // should close with }My questions:
- Is pushing each opening bracket onto a stack a practical way of doing this? I feel that my approach with it isn't too practical as it involves a lot of conditionals. When a closing bracket is found, there has to be some way to determine if they match properly.
- Would an
std::mapbe beneficial in serving as a look-up table for associating the opening and closing brackets with each other? I feel that it may help with DRY here.
- Although it may seem easy to maintain all the error-checking in one function, should they still be split into separate functions? If so, should the messages be displayed in them or in
main()?
- Does it make sense to return
EXIT_SUCCESSif the program terminated from a matching error but not a file error? I'm not sure if I should returnEXIT_FAILUREinstead, even though it already does that if the file cannot be opened.
I don't mind following an entirely different procedure if this one isn't very practical. If you have something more complicated in mind, please share it. I want to approach this the right way.
```
#include
#include
#include
#include
#include
type
Solution
I'd rewrite it with three main changes:
-
Keep
By the Single Responsibility Principle, it's a good idea to limit
Being a Unix/Linux user, I would prefer to see tools that adhere to some Unixy conventions:
In the context of this program, I would consider failure to open the input file as an error condition to be reported to
To answer your question 4 about the exit status of the program: 0 means success; beyond that there is no universal convention.
-
Use
That saves you from the tedium of iterating character by character.
-
Keep the expected closing delimiters in the stack.
That seems to reduce redundancy in the code. This addresses your questions 1 and 2.
In addition, I've enhanced it to keep track of line and column numbers to help find the location of the mismatch. The diagnostics could be even more informative if you kept track of the location of every delimiter you push onto the stack.
-
Keep
main() minimal and improve the user interface.By the Single Responsibility Principle, it's a good idea to limit
main() to just calling the primary function with the appropriate parameters. In this case, the functionality splits very cleanly.Being a Unix/Linux user, I would prefer to see tools that adhere to some Unixy conventions:
- Take input from a file named on the command line, or from standard input if there is no command-line parameter.
- On success, remain silent and return 0 (unless you add support for a
--verboseflag, which I haven't bothered to implement).
In the context of this program, I would consider failure to open the input file as an error condition to be reported to
std::cerr, and a delimiter mismatch to be normal output to be reported to std::cout. (To answer your question 3, I've just taken the easy route and printed the errors as I encounter them. That may not be the most elegant method, but it saves me the trouble of encoding the mismatch into some kind of representation. When in doubt, keep it simple, I think.)To answer your question 4 about the exit status of the program: 0 means success; beyond that there is no universal convention.
-
Use
std::string::find_first_of().That saves you from the tedium of iterating character by character.
-
Keep the expected closing delimiters in the stack.
That seems to reduce redundancy in the code. This addresses your questions 1 and 2.
In addition, I've enhanced it to keep track of line and column numbers to help find the location of the mismatch. The diagnostics could be even more informative if you kept track of the location of every delimiter you push onto the stack.
#include
#include
#include
#include
#include
#include
bool bracketsMatch(std::istream &in) {
std::stack expectedDelimiters;
int lineNum = 0;
std::string line;
while (std::getline(in, line)) {
lineNum++;
size_t pos = 0;
while (std::string::npos != (pos = line.find_first_of("(){}[]", pos))) {
int colNum = pos + 1;
switch (line[pos]) {
case '(': expectedDelimiters.push(')'); break;
case '{': expectedDelimiters.push('}'); break;
case '[': expectedDelimiters.push(']'); break;
case ']':
case '}':
case ')':
if (expectedDelimiters.empty()) {
std::cout 1) ? (f.open(argv[1]), f) : std::cin;
if (!in) {
std::cerr << argv[0] << ": " << argv[1] << ": "
<< std::strerror(errno) << std::endl;
return 2; // A rather arbitrary error code
}
return bracketsMatch(in) ? EXIT_SUCCESS : EXIT_FAILURE;
}Code Snippets
#include <cstdlib>
#include <cstring>
#include <fstream>
#include <iostream>
#include <stack>
#include <string>
bool bracketsMatch(std::istream &in) {
std::stack<char> expectedDelimiters;
int lineNum = 0;
std::string line;
while (std::getline(in, line)) {
lineNum++;
size_t pos = 0;
while (std::string::npos != (pos = line.find_first_of("(){}[]", pos))) {
int colNum = pos + 1;
switch (line[pos]) {
case '(': expectedDelimiters.push(')'); break;
case '{': expectedDelimiters.push('}'); break;
case '[': expectedDelimiters.push(']'); break;
case ']':
case '}':
case ')':
if (expectedDelimiters.empty()) {
std::cout << "Mismatched " << line[pos]
<< " at line " << lineNum << ", col " << colNum
<< std::endl;
return false;
}
if (line[pos] != expectedDelimiters.top()) {
std::cout << "Expected " << expectedDelimiters.top()
<< ", found " << line[pos]
<< " at line " << lineNum << ", col " << colNum
<< std::endl;
return false;
}
expectedDelimiters.pop();
}
pos = colNum;
}
}
// Should check for a possible input error here, but I didn't bother.
if (!expectedDelimiters.empty()) {
std::cout << "Expected " << expectedDelimiters.top()
<< " at end of file" << std::endl;
return false;
}
return true;
}
int main(int argc, const char *argv[]) {
// The command-line parsing below is a bit sloppy (no validation,
// help message, or option handling, for example).
std::ifstream f;
std::istream &in = (argc > 1) ? (f.open(argv[1]), f) : std::cin;
if (!in) {
std::cerr << argv[0] << ": " << argv[1] << ": "
<< std::strerror(errno) << std::endl;
return 2; // A rather arbitrary error code
}
return bracketsMatch(in) ? EXIT_SUCCESS : EXIT_FAILURE;
}Context
StackExchange Code Review Q#40515, answer score: 18
Revisions (0)
No revisions yet.