trying to sort a simple string in c++

Question

#include "stdio.h"
#include "conio.h"
#include <iostream>

using namespace std;

int main (void) 
{
    char my_char[] = "happy birthday";

    int i;

    bool j=false;
    char my_char_temp[1];

    do
    {
        for (i=0;i<sizeof(my_char)-2;i++)
        {
            j=false;
            if (my_char[i+1] < my_char[i])
            {
                my_char_temp[0]=my_char[i+1];
                my_char[i+1] = my_char[i];
                my_char[i] =  my_char_temp[0];
                j=true;
            }
        }

    }while (j);

    cout << my_char;
}

What am I doing wrong?

I'm just trying to sort the letters within the char.

The output I get is completely wrong.

Answer 1

I don't know what are you trying to implement with your sizeof(...) - 2 and etc, but what you probably want to get can be done this way:

#include <iostream>
#include <algorithm>

int main() {
   std::string s("happy birthday");
   std::sort(s.begin(), s.end());
}

Answer 2

Consider what happens inside this loop:

for (i=0;i<sizeof(my_char)-2;i++)

If you find a pair of values to swap, setting j to true, you'll continue iterating through that loop, and set j back to false on the next iteration. As a result, the program is going to exit as soon as the last two characters in the string are in sorted order, regardless of whether the rest of the string is sorted.

Instead, as soon as you find a pair of characters to swap, you want to start over again at i=0. The simplest way to do that is add a break; statement after your j = true line. With that fix, this works correctly.

Alternately, you could move the initial j = false line outside the loop, which would solve the problem in a slightly different way.

Answer 3

You are resetting j to false each and every time you compare two characters.

This means that, if you swap two characters, and you are NOT at the end of your array, you will forget that you have swapped them.

Move the j=false; from inside the for-loop to just inside the do-loop.

And you owe me a bottle of Jack for saving your ass on a homework assignment on Sunday afternoon.

Answer 4

You are actually very close. The only problem is that

        j=false;

needs to be in the outer loop. As is, j is cleared every time the inner loop executes.

With this fix, your program works fine for me.

Stylistic errors, however, are another story.

Answer 5

You want to use strlen() rather than sizeof.

Answer 6

I could be mistaken but it looks like you're trying to do a bubble sort? And it's i < sizeof(my_char)-2 because he's using a 0-based, null terminated string, and he doesn't want to sort the null terminator.

Try just repeating the condition of the inner loop, using j instead of i, and see if that works? Note that this has a run time of O(n^2) and you can get sorts down much much faster than that if you need to. Alternately you can move the boolean out of the for and into the do loop.

for (i=0;i < sizeof(my_char)-2;i++)
            for (i=0;i<sizeof(my_char)-2;i++)
        {

            if (my_char[i+1] < my_char[i])
            {
                my_char_temp[0]=my_char[i+1];
                my_char[i+1] = my_char[i];
                my_char[i] =  my_char_temp[0];

            }
}