balanced bracket C++ not working, what am i doing wrong

Question

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

main()
{
    int i, j=0, perlen, countcp=0, countsp=0, countrp=0, countcl=0, countsl=0, countrl=0;
    string str, str1;
    cout<<"Please enter string"<<endl;
    getline(cin, str);
    perlen=(str.length())/2;
    for(i=0; i<str.length(); i++)
    {
        if(str[i]=='{')
            countcp++;  
        if(str[i]=='[')
            countsp++;
        if(str[i]=='(')
            countrp++;
        if(str[i]=='}')
            countcl++;
        if(str[i]==']')
            countsl++;
        if(str[i]==')')
            countrl++;
    }
    str1=str;

    if(countcp==countcl and countsp==countsl and countrp==countrl)
    {
        cout<<"equal"<<endl;
        int countwhile=0, j=0;
        while(!str.length()==0)
        {
            if(str[j]=='{' and str[j+1]=='}')
            {
                str.erase(i, 2);
                countwhile++;
            }
            else if(str[j]=='(' and str[j+1]==')')
            {
                str.erase(i, 2);
                countwhile++;
            }
            else if(str[j]=='[' and str[j+1]==']')
            {
                str.erase(i, 2);
                countwhile++;
            }
            if(countwhile>perlen)
            {
                countwhile=1;
                cout<<"reached break"<<endl;
                break;
            }
            j++;
        }
        if(countwhile==1)
        {
            cout<<"Balanced string "<<str1<<endl;
        }
    }
}

i am trying to balance brackets. input will include curly, round and square brackets. i am trying to find what i did wrong in this code. i am new to c++ and i am trying to learn.

explanation

countcp for curly open brackets
countsp for square open brackets
countrp for round open brackets
countcl for curly close or last bracket open brackets
countsl for square close brackets
countrl for round close brackets
eg. input {()}
output balanced
input {(}{)}
output not balanced it works till line 30 and prints equal after that it gives error Segmentation fault (core dumped)

You should inspect your code using the debugger and step through your code line by line. That's probably more efficient than asking an unclear question here. — πάντα ῥεῖ, Jan 11 '16 at 07:08
you are checking only for `str[j]` and `str[j+1]`. I suggest better use stack for this que. — roottraveller, Jan 11 '16 at 07:11

score 0 · Answer 1 · answered Jan 11 '16 at 07:27

0

This line

str.erase(i, 2);

seems wrong as i is not part of the loop. i is equal str.length(); which causes the erase to fail.

Did you mean

str.erase(j, 2);

answered Jan 11 '16 at 07:27

Support Ukraine

42,271
4
38
63

yes, i mean j thnx for help but now i am getting an error terminate called after throwing an instance of 'std::out_of_range' what(): basic_string::erase Aborted (core dumped) – Dementor Jan 11 '16 at 07:35
I think you need to set `j` back to zero after each erase. Or perhaps set it to `-1` as you have a `j++` later. The idea must be to start all over whenever you erased a matching pair. – Support Ukraine Jan 11 '16 at 07:43
yes tht was the issue i forgot to make j zero. thnx now it is working – Dementor Jan 11 '16 at 07:56

R Sahu · Accepted Answer · 2016-01-11T07:50:35.273

Problems I see:

Missing return type in main. Use:
```
int main() ...
```
Accessing array out of bounds. You have:
```
while ( !str.length() == 0 ) 
```
That is not adequate. You also need to make sure that you don't access str out of bounds. Hence, change that to:
```
while ( !str.length() == 0 && j+1 < str.length() ) 
```
or, better,
```
while ( !str.empty() && j+1 < str.length() ) 
```
Use of j+1 is necessary since you are accessing str[j+1] in the loop.

Erasing the wrong elements from the string. Instead of

str.erase(i, 2);
//        ^^

use

str.erase(j, 2);
//        ^^

You are not updating the value of j properly. Say str is equal to "{()}". Whenjis equal to1, you are trying to remove()from the string. After that,stris equal to"{}". In order to be able process that, you need to set the value of j to 0. The logic needs to be:

Increment j when there is no match.
Decrement j when there is a match and the matching characters are removed.

Suggestion for improvement

Use an unsigned type for i and j to avoid compiler warnings.

You can use:

std::string::size_type i = 0;
std::string::size_type j = 0;

Updated version of the while loop with the above fixes

I have also added additional cout lines to help diagnose logic errors.

  int countwhile=0;
  std::string::size_type j=0;
  while(!str.length()==0 && j+1 < str.length())
  {
     if(str[j]=='{' and str[j+1]=='}')
     {
        cout<<"erasing {}"<<endl;
        str.erase(j, 2);
        cout << str << endl;
        countwhile++;
        --j;
     }
     else if(str[j]=='(' and str[j+1]==')')
     {
        cout<<"erasing ()"<<endl;
        str.erase(j, 2);
        cout << str << endl;
        countwhile++;
        --j;
     }
     else if(str[j]=='[' and str[j+1]==']')
     {
        cout<<"erasing []"<<endl;
        str.erase(j, 2);
        cout << str << endl;
        countwhile++;
        --j;
     }
     else
     {
        j++;
     }
     if(countwhile>perlen)
     {
        countwhile=1;
        cout<<"reached break"<<endl;
        break;
     }
  }

In order to be able process input like "{[}{]}" correctly, you have to restructure the code a bit. This does not seem like the right place to suggest an elaborate refactoring of your code to be able to handle such input.

Thank-you very much now it works. i am still testing it for larger strings. — Dementor, Jan 11 '16 at 07:54
after all the edits it works for ({}). but doesnt work for ({({}})). — Dementor, Jan 11 '16 at 07:58
@Dementor, you will need to use a different strategy and algorithm for dealing with such inputs. You'll have to spend some time working through such input in pencil and paper before working on updating the program. — R Sahu, Jan 11 '16 at 08:02
@R Sahu, i am matching adjacent brackets they why is it failing — Dementor, Jan 11 '16 at 08:03
@Dementor, My suggestion: Look for the closing characters. If you find one such character, check the value of the previous character. If that is not a matching character, then there is a problem. Otherwise, it's OK. — R Sahu, Jan 11 '16 at 15:59

score 0 · Answer 3 · answered Jan 11 '16 at 08:13

so thw while should be like this

while(!str.length()==0 and j+1 < str.length())
            {
                     if(str[j]=='{' and str[j+1]=='}')
                    {
                        str.erase(j, 2);
                        countwhile++;
                        j--;
                    }
                else if(str[j]=='(' and str[j+1]==')')
                    {
                        str.erase(j, 2);
                        countwhile++;
                        j--;
                    }
                else if(str[j]=='[' and str[j+1]==']')
                    {
                        str.erase(j, 2);
                        countwhile++;
                        j--;
                    }
                else
                    {
                        j++;
                    }
                if(countwhile>perlen)
                    {
                        countwhile=1;
                        cout<<"reached break"<<endl;
                    }
                if(countwhile==1)
                    {
                        cout<<"Balanced string "<<str1<<endl;
                        break;
                    }
                else
                    {
                        cout<<"not Balanced "<<str1<<endl;
                        break;
                    }
            }

balanced bracket C++ not working, what am i doing wrong

3 Answers3