patterncppMinor
Valid Parentheses
Viewed 0 times
parenthesesvalidstackoverflow
Problem
Given a string containing just the characters '(', ')', '{', '}', '['
and ']', determine if the input string is valid.
The brackets must close in the correct order, "()" and "()[]{}" are
all valid but "(]" and "([)]" are not.
The following is my code, which has already been tested on a onlinejudge.
Can someone help me give some comments for the code itself?
and ']', determine if the input string is valid.
The brackets must close in the correct order, "()" and "()[]{}" are
all valid but "(]" and "([)]" are not.
The following is my code, which has already been tested on a onlinejudge.
Can someone help me give some comments for the code itself?
bool isValid(string s) {
stack s1;
string::iterator it = s.begin();
map x;
x['('] = ')';
x['['] = ']';
x['{'] = '}';
for (; it!=s.end(); ++it) {
if (!s1.empty()) {
char c = s1.top();
if(x[c] == *it) {
s1.pop();
}
else {
s1.push(*it);
}
}
else {
s1.push(*it);
}
}
return s1.empty();
}Solution
You use of objects without prefix them with
I bet you used
https://stackoverflow.com/q/1452721/14065
When passing objects to functions prefer to pass by
To prevent you accidentally modifying things try and use cost_itrerator when you need only read only accesses to a container:
In C++11 you can use the new 'brace init' feature:
Also because this is a imutable you can make it
std:: is a bad sign.I bet you used
using namespace std; in the code. Please stop this habit.https://stackoverflow.com/q/1452721/14065
When passing objects to functions prefer to pass by
const reference to prevent unnecessary copies.bool isValid(std::string const& s) {
// ^^^^^ ^^^^^^To prevent you accidentally modifying things try and use cost_itrerator when you need only read only accesses to a container:
std::string::const_iterator it = s.begin();In C++11 you can use the new 'brace init' feature:
map x;
x['('] = ')';
x['['] = ']';
x['{'] = '}';
//replace with:
map x = {{'(', ')'}, {'[',']'},{'{','}'}};Also because this is a imutable you can make it
const and static. The const will help prevent errors while the static is so you only create it once.static map const x = {{'(', ')'}, {'[',']'},{'{','}'}};Code Snippets
bool isValid(std::string const& s) {
// ^^^^^ ^^^^^^std::string::const_iterator it = s.begin();map<char, char> x;
x['('] = ')';
x['['] = ']';
x['{'] = '}';
//replace with:
map<char, char> x = {{'(', ')'}, {'[',']'},{'{','}'}};static map<char, char> const x = {{'(', ')'}, {'[',']'},{'{','}'}};Context
StackExchange Code Review Q#24208, answer score: 4
Revisions (0)
No revisions yet.