Fixes issue #3263-Code implementation in C++ for balanced_expression in stack data structure #3624
Conversation
Issue OpenGenus#3263- C++ for balanced_expression
| #include <cstring> | ||
| #include <stack> | ||
|
|
||
| using namespace std; |
|
|
||
| bool isOpen(char c) | ||
| { | ||
| if(c == '(' || c == '{' || c == '[') |
There was a problem hiding this comment.
How about: return c == '(' || c == '{' || c == '[';? You could also consider marking this function inline.
There was a problem hiding this comment.
As the return type is bool so we need to return true or false and not 1 or 0. I have checked that the code fails in that case.
|
|
||
| bool isBalanced(char popped, char now) | ||
| { | ||
| if(popped == '(' && now ==')') |
There was a problem hiding this comment.
return (popped == '(' && now ==')') ||
(popped == '[' && now ==']') ||
(popped == '{' && now =='}');There was a problem hiding this comment.
As the return type is bool so we need to return true or false and not 1 or 0. I have checked that the code fails in that case.
There was a problem hiding this comment.
@bahl24 Sorry what? Last I checked, the or operator returns a bool value. 1 and 0 evaluate to true and false respectively anyway. This shouldn't be a problem, unless the return type was that of an integer (it isn't). Could you clarify what test cases this fails in?
There was a problem hiding this comment.
There was a problem hiding this comment.
Actually, it does output NO. Check this and modify as you desire (write #define ARNAVB_VERSION at the beginning of the file to check my version): https://wandbox.org/permlink/ygACUgdFsVMLdG8Q
As for your second point, I personally disagree, since it is repetitive, but it is a matter of opinion so I'm not going to contradict you there. However, the code I gave definitely works, as both snippets are logically equivalent, as shown in the wandbox above (I used the sample input you gave).
| { | ||
| /* The code takes a string str as input from stdin and prints "YES" | ||
| if str is a balanced expression else it prints "NO".*/ | ||
| stack<char> s; |
There was a problem hiding this comment.
Fix your indentation here please.
| if str is a balanced expression else it prints "NO".*/ | ||
| stack<char> s; | ||
| string str; | ||
| cin>>str; |
There was a problem hiding this comment.
Add spaces around operators please.
| string str; | ||
| cin>>str; | ||
| int no = 0; | ||
| for(int i = 0; i < str.length(); i++) |
There was a problem hiding this comment.
I think a range based for loop might be clearer, unless i is used somewhere in the loop.
There was a problem hiding this comment.
As i is being used inside the loop, a range based loop will lead in more execution time or might be insufficient if str is quite long. Plus using str.length() makes viewer exactly understand the code.
| } | ||
| } | ||
| } | ||
| if(s.size() != 0) |
|
@arnavb I have reviewed the changes suggested by you and have made the changes wherever applicable. I have also justified my reasons few changes suggested. |
| #include <iostream> | ||
| #include <stack> | ||
|
|
||
| using namespace std; |
There was a problem hiding this comment.
Just remove usage of using namespace std, and you're good to go!
Fixes issue:
Fixes issue #3263-Code implementation in C++ for balanced_expression in stack data structure.
Changes:
A C++ implementation for balanced_expression was added. This expands the language base for the Stack data structure for which currently only Java implementation is available.