0

I am struggling with this code, i cant seem to figure this out. Heres the question, Write a program that usues a single stack to check wether a string containing braces, parenthesis, and brackets is properly delimted? this is what i wrote so far but i cant seem to see it. There are no errors and it does compile but it crashes after I enter my string. How do I fix it?

#include <iostream>
#include <string>
#include <stack>
using namespace std;

bool isbalanced(string);

int main()
{
    string s;

    cout<< "This program checks to see if the delimiters are properly balanced" << endl;
    cout << "Enter a string with some paranthesis" << endl;
    getline(cin, s);

    if (isbalanced(s))
        cout << "The string has balanced paranthesis" << endl;
    else
        cout << "String does not have balanced parathesis" << endl;

    return 0;
}
bool isbalanced(string s)
{
    stack<char> stack;
    char ex;
    for (unsigned int k = 0; k = s.length(); k++)
    {
        switch (s[k])
        {
        case '(': stack.push(')');
            break;
        case'{': stack.push('}');
            break;
        case '[': stack.push(']');
            break;
        case ')':
        case '}':
        case ']':
            ex = stack.top();
            stack.pop();
            if (ex != s[k])
            {
                return false;
            }
            break;
        deafult:break;
        }
    }
    if (stack.empty())
        return true;
    else
        return false;
Asteroids With Wings
  • 17,071
  • 2
  • 21
  • 35
ThanosCheeks
  • 7
  • 4
  • dont worry i have #include I just couldn't add it to the question for some reason – ThanosCheeks Apr 02 '20 at 18:13
  • 1
    `for (unsigned int k = 0; k = s.length(); k++)` should be `for (unsigned int k = 0; k < s.length(); k++)`. `s.size()` is more idiomatic, but `s.length()` works as well. – JohnFilleau Apr 02 '20 at 18:19
  • 1
    `k = s.length()` assigns k instead of comparing it and it counts one too far. it should be `k == (s.length() - 1)` or `k < s.length()` – perivesta Apr 02 '20 at 18:21
  • When you write code, start with something simple that works perfectly, then build up in small steps. You shouldn't have attempted parentheses, brackets and braces all at once; get *one* working, then add the other two. – Beta Apr 02 '20 at 18:26
  • This doesn't address the question, but `if (stack.empty()) return true; else return false;` can be written much more simply as `return stack.empty();`. – Pete Becker Apr 02 '20 at 18:28
  • `=` is assignment in C and C++. – David Schwartz Apr 02 '20 at 18:29
  • so my code is ugly @JohnFilleau and how can I improve it if possible – ThanosCheeks Apr 02 '20 at 19:01
  • @Thanos I don't think it's particularly ugly. What part is ugly to you? – JohnFilleau Apr 02 '20 at 19:13
  • when testing your code instead of standard input you can use a hard-coded string. However JohnFilleau and dave have pointed out the bug – Dr Phil Apr 02 '20 at 19:24

2 Answers2

2

You are reaching behind the end of the string in the for loop, try:

for (unsigned int k = 0; k < s.length(); k++)

Also consider passing const std::string& instead of copying the parameter.

And remove using namespace std;

Ardent Coder
  • 3,777
  • 9
  • 27
  • 53
Pawel Kaczanowski
  • 46
  • 6
  • Noting the `const` may be beneficial I don't think you should inist on removing `std`. While it is extremely common practice to not add that `using`, it is still an opinion and irrelevant to this question. – Derek C. Apr 02 '20 at 18:36
  • 1
    @DerekC. [`using namespace std` is generally accepted to be bad practice.](https://stackoverflow.com/q/1452721/9254539). Just because it's not causing the problem doesn't mean we can't teach people to use good practice. – eesiraed Apr 02 '20 at 18:45
  • Passing by non-const reference may indicate that the string is going to be modified inside the function. In my opinion it is a good habit to put ```const``` wherever possible. – Pawel Kaczanowski Apr 02 '20 at 18:48
  • @DerekC. Even if `using namespace std` is okay sometimes you end up with people who will put that on the top of every single file because they have always been taught this way and they don't know any better. – eesiraed Apr 02 '20 at 18:51
  • so I should not use namespace std – ThanosCheeks Apr 02 '20 at 18:54
  • ThanosCheeks no, you can look up why not (which you should to understand why). I was simply stating it was irrelevant to the question at hand. @BessieTheCow of course, I'm well aware it's bad. At the very least it shouldn't be "remove it" and should have "because...". Best practice doesn't work if you don't understand why you are doing it. – Derek C. Apr 02 '20 at 19:01
1

The problem is with the for loop. Specifically the second part of it.
You are assigning the value of k rather than comparing it to something.
k = s.length() the one equal sign is an assignment.
You probably meant to do k < s.length()

I wanted to create my own answer to highlight the fact that you can compile because the code is correct, assigning a variable does yield a truthful boolean. Then in the loop it accesses s[s.length()] which is out of bounds seeing as arrays (like a string) start at 0 rather than 1. The length is one more than the final index because that's what the actual length is.

Derek C.
  • 890
  • 8
  • 22