1

User enters a string like '3+2+1' or '1+3+2+1+3+1'. And I just have to sort the string. But after 4-5 test cases error shows up.

Input  - 2
Output - �

Input  - 2+1+2+2+2+3+1+3+1+2
Output - �+1+1+1+2+2+2+2+2+3
#include<iostream>
using namespace std;

int main()
{

    string s;
    cin>>s;

    for(int i=0;i<s.size();i+=2)
    {
        for(int j=0;j<(s.size()-i);j+=2)
        {
            if(s[j]>s[j+2])
            {
                swap(s[j],s[j+2]);
            }
        }
    }

    cout<<s;
    return 0;
}

Bill Lynch
  • 80,138
  • 16
  • 128
  • 173
logan007
  • 67
  • 1
  • 10
  • 2
    Welcome to SO! Your inner loop runs to `size` and inside this loop you access `size-1+2` which is out of bounds. What are you trying to accomplish here? If you want to sort the string alphabetically, use [`std::sort`](https://stackoverflow.com/a/9107552/6243352). – ggorlen Aug 28 '20 at 05:38
  • 1
    Here's a tip, instead of using `s[j]` use `s.at(j)`. There difference is that if `j` is too big for `s` with `at` you will get an exception thrown, so you will know there is a problem. With `s[j]` the results are unpredictable if `j` is too big. – john Aug 28 '20 at 05:43
  • Or turn on your standard library's debug mode, which will likely give you a hard error at that indexing call. Or compile with address sanitizer to catch memory errors that might slip through the cracks. – chris Aug 28 '20 at 05:48
  • It seems that you are studying a bubble sort implementation, but consistently got the increase (usuall `i++` or `i+=1` or ì=i+1`) wrong and increase too much, i.e. by 2. Could you explain that very striking oddity of your code? Without that the question is unclear. – Yunnosch Aug 28 '20 at 06:00
  • @Yunnosch I think `+2` is because the OP wants to sort only the digits of the expression (and consequently they are ignoring the possibility of a muti digit number). – john Aug 28 '20 at 08:18
  • BTW, you might find this interesting: https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice – Stef Aug 28 '20 at 09:06
  • @john Very plausible (and I did not see it...). I'd like to know OPs thinking. – Yunnosch Aug 28 '20 at 10:08
  • @Yunnosch +2 was to avoid the plus sign and only consider the digits. – logan007 Aug 28 '20 at 15:55
  • @john Thanks a lot. I got to know the problem use s.at(). – logan007 Aug 28 '20 at 15:57

2 Answers2

2

As ggorlen said, garbage is showing up because you're accessing a value that's out of bounds. Try checking if your "j" index plus 2 is out of bounds before doing any swap.

#include<iostream>
using namespace std;

int main()
{

    string s;
    cin>>s;

    for(int i=0;i<s.size();i+=2)
    {
        for(int j=0;j<(s.size()-i);j+=2)
        {
            if(j+2 < s.size() && s[j]>s[j+2])
            {
                swap(s[j],s[j+2]);
            }
        }
    }

    cout<<s;
    return 0;
}

Input 2+1+2+2+2+3+1+3+1+2

Output 1+1+1+2+2+2+2+2+3+3

Injourn
  • 71
  • 6
0

Value of j is going out of bounds. Try using insertion sort:

#include <bits/stdc++.h>
using namespace std;

void insertionSort(string s)
{
    int key, j; 
    for (int i = 1; i < s.size(); i++) 
    { 
        key = s[i]; 
        j = i - 1; 
        while (j >= 0 && s[j] > key) 
        { 
            s[j + 1] = s[j]; 
            j = j - 1; 
        } 
        s[j + 1] = key; 
    } 
    for (int i = 0; i < s.size(); i++) 
        cout << s[i] << " "; 
    cout << endl; 
}

int main() 
{ 
    string s;
    cin>>s;
    insertionSort(s); 

    return 0; 
}

Hope you understand the code.

NiteLite
  • 41
  • 5