Need Help Finding Error In Select Sort Routine

[rory.starkweather.7]

This only appears to move one character even though it does a lot of thrashing around when I step through it. It looks like it's only executing the loops once, but the topmost character in the array is corect. It just like the others aren't moved at all.

#include <string.h>
#include <stdio.h>
#include <stdlib.h>

void select(char *item, int count);

int main(void)
{
    char s[80];

    system("cls");

    printf ("Enter a string: ");
    gets(s);
    select (s, strlen(s));
    printf ("The sorted string is: %s\n", s);
    return 0;
}

/*  Select Sort  */
void select(char *item, int count)
{
    register int a, b, c;
    char t;

    for (a = 0; a < count - 1; ++a)
        {
         c = a;
         t = item[a];
         for (b = a + 1; b < count; ++b)
            {
             if (item[b] < t);
                {
                 c = b;
                 t = item[b];
                }
            }
         item[c] = item[a];
         item[a] = t;
        }
}

I'm brushing up on my troubleshooting skills right now, but I just can't see the problem. However this refresher training for me. I haven't done any C programming for 20 years.

The box has an AMD 3+ quad core processor running WinXPSP3. The C package is Borland Turbo C 2.01.

EDIT: I forgot to say that it compiles and links with no errors or warnings, and I'm using the Small memory model.

[Ancient Dragon]

Where did you get that algorithm? Here's another for you to study.

[rory.starkweather.7]

That one came out of 'Advanced Turbo C', 2nd Edition, by Herbert Schildt, who I've never really liked, although he wrote most of the computer books I have. I'll be happy to look at yours.

It's funny. Just the other day I mentioned to someone looking for advice that I thought many prospective programmers have been driven away from the field by poorly written books. I don't know how many times I've got a new book and not even been able to get the first example running. Every time I wonder if I'm really cut out for this. Especially when you are new, or have forgotten everything you ever knew, you have to assume that the book is right. Well, it would be nice if that was true.

[Ancient Dragon]

Pogramming books, especially older ones, sometimes contain errors. Go the the publisher's web site and see if it has free download of the source code in the book which will generally have all the bugs fixed. Another good source is google, for example good for "c++ selection sort".

[rory.starkweather.7]

I've decided that it's time to see if I've learned anything. It's not a big program, and I know exactly how it is supposed to work, so I'm going to write my own . . . which will probably look a lot like any of the others that work. It's time to stop doing examples from the books and move out on to the bleeding edge again. Just to make it more fun, I plan to have it finished by morning. That's the kind of challenge that really makes programming interesting for me. Come to think of it, it won't look like anyone else's because I plan to use meaningful variable names in camel case. :)

[Ancient Dragon]

Do you mean selection sort algorithm? There are lots of sorting algorithms, some better than others.

[rory.starkweather.7]

I know. That's what the current chapter is about. Tis is number 3 of I don't know how many.

Want to see something wierd? I'll post the code first and attach 2 screenshots of the output. I can also see why C programmers might want to use single letter variable names now.

#include <string.h>
#include <stdio.h>
#include <stdlib.h>

void select(char *p2caInput, int intCount);

int main(void)
{
    char s[80];

    system("cls");

    printf ("Enter a string: ");
    gets(s);
    select (s, strlen(s));
    printf ("The sorted string is: %s\n", s);
    return 0;
}

/*  Select Sort  */
void select(char *p2caInput, int intCount)
{
    register int rintIndex, rintLoopCount, rintMoveDownTo;
    char chrLowest;

    chrLowest = p2caInput[0];
    for (rintLoopCount = 0; rintLoopCount < intCount - 1; rintLoopCount++)
        {
         for (rintIndex = rintLoopCount; rintIndex < intCount - 1; rintIndex++)
            {
             if (p2caInput[rintIndex] < chrLowest)
                {
                 rintMoveDownTo = rintIndex;
                 chrLowest = p2caInput[rintIndex];
                }
            }
         p2caInput[rintMoveDownTo] = p2caInput[rintLoopCount];
         p2caInput[rintLoopCount] = chrLowest;
         chrLowest = p2caInput[rintLoopCount + 1];
        }
}

Hmmm. It won't let add attachments. I'll type it in:
Input: qwertyuiopasdfghjklzxcvbnmMNBVCXZLKJHGFDSAPOIUYTREWQ
Output:
ABCDDEFGHIJKLMNOPRSTUVWXYZabcdefghijklmnoppqstuvwxyyQ

I think I can fix the capital Q in a minute but what happened after the small o?

[Ancient Dragon]

for (rintIndex = rintLoopCount; rintIndex < intCount -1; rintIndex++)

That loop on line 29 is incorrect. don't subtract 1 from intCount

for (rintIndex = rintLoopCount; rintIndex < intCount; rintIndex++)

This is the output I got, but it too has a problem (double e)

The sorted string is: ABCDEFGHIJKLMNOPQRSTUVWXYZabcdeefghijklmnopqrstuvxyz

[Ancient Dragon]

Here (click here) is a working select sort

void selectSort(char* arr, int n)
{
    //pos_min is short for position of min
    int pos_min, temp;

    for (int i = 0; i < n - 1; i++)
    {
        pos_min = i;//set pos_min to the current index of array

        for (int j = i + 1; j < n; j++)
        {

            if (arr[j] < arr[pos_min])
                pos_min = j;
            //pos_min will keep track of the index that min is in, this is needed when a swap happens
        }

        //if pos_min no longer equals i than a smaller value must have been found, so a swap must occur
        if (pos_min != i)
        {
            temp = arr[i];
            arr[i] = arr[pos_min];
            arr[pos_min] = temp;
        }
    }
}

[rory.starkweather.7]

That helped a lot. Only two problems left. with the same input the capital letters are fine, but the small letters are:abcdeefghijklmnopqrstuvxyz. Two 'e's and no w. Hmmm 'w' is the second letter of the input and 'e' is the third.

[rory.starkweather.7]

It didn't take very long to do. I think I'll just start over. I learned a lot from that try. I've got real problems figuring out the loop controls, though. I need to work on that.

This time I'll use short variable names. Maybe that will help. :)

[Ancient Dragon]

Yes, for loop control one-letter variable names are ok, for example common ones are i, j, k, and m.

[rory.starkweather.7]

How about this one?

#include <string.h>
#include <stdio.h>
#include <stdlib.h>

void select(char *p2ca, int count);

int main(void)
{
    char s[80];

    system("cls");

    printf ("Enter a string: ");
    gets(s) ;
    select (s, strlen(s));
    printf ("The sorted string is: %s\n", s);
    return 0;
}

/*  Select Sort  */

/*  Variable Name Key
lc    = Lowest Character
tc    = Transfer Character
riolc = Register Integer Outer Loop Counter
riilc =    "        "    Inner  "      "
rixfr =    "        "    Transfer Variable
p2ca  = Pointer To Character Array
count = number of characters actually in the array
*/

void select(char *p2ca, int count)
{
    register int riolc, riilc, rixfr;
    char lc;

    lc = p2ca[0];

    for (riolc = 0; riolc <= count; ++riolc)
        {
         for (riilc = riolc; riilc < count; ++riilc )
            {
             if ( p2ca[riilc] < lc )
                {
                 lc = p2ca[riilc];
                 rixfr = riilc;
                }
            }
         if ( lc != p2ca[riolc])
            {
             p2ca[rixfr] = p2ca[riolc];
             p2ca[riolc] = lc;
            }
         lc = p2ca[riolc + 1];
        }

}

It looks to me like it works. How much do you think that 'if' statement will slow it down? The one near the end.

[Ancient Dragon]

On today's fast computers, probably not at all, or at least not enough to be measurable. If you are using a really old and slow computer you can profile it to find out how it performs on your computer.

[rory.starkweather.7]

I'm going to try it without the if statement. I think I fixed the problem I needed it for when I finally got the loops straightened out.

;))))) It still works.

[rory.starkweather.7]

Akkk! On the second run I got two 'e's and two'p's. And no 'w'.