views:

372

answers:

3

Can you tell me what's wrong with my method? I ends up putting the same thing everywhre and it's actually not sorting.

void sortArrays(){

    int i, j;



    for(i=0; i<counter; i++){



        for( j=0; j<i; j++){

            if( strcmp(title_arr[i], title_arr[j]) < 0){

                char* title_temp = title_arr[i];

                title_arr[j] = title_temp;





            }

        }

    }
+10  A: 

This:

char* title_temp = title_arr[i];

title_arr[j] = title_temp;

Is equivalent to:

title_arr[j] = title_arr[i];

You never swap them, you just copy one to the other. You should add this line:

title_arr[i] = title_arr[j];

In between the two. That way, you'll overwrite [i] with [j], but _temp still holds the old value of [i], so you can copy that value into [j], thus swapping them.

I suppose it's also a time for a lesson on algorithms. Your algorithm is known as a "bubble sort" algorithm. It is known for it's simplicity, but in a realistic setting it is known for it's inefficiency (the technical term is "teh sux", and the real technical term is O(n^2) ("N squared") performance). Some more common (and more efficient) algorithms include Quicksort, merge sort, and Heapsort, among many others. For more about measuring algorithmic scalability, see the article on Big Oh notation.*

But, as vava pointed out in a comment, unless your assignment is to write your own sorting function, you're going to get better performance with qsort (in C) or std::sort (in C++).

int mystrsort(const void *a, const void *b)
{
    return strcmp(*(const char **)a, *(const char **)b);
}

// later:
qsort(title_arr, sizeof title_arr / sizeof(char *), sizeof(char *), mystrsort);

I'm not going to stab at std::sort, but it's going to work about the same (perhaps easier).**

*Note that anyone who likes is free to change these Wikipedia links to Stack Overflow links. It would be better to link to SO, I just linked to Wikipedia because I knew how to find the info I needed faster.
**Note that anyone who likes is free to add a std::sort example. I'm just not sufficiently familiar with C++.

Chris Lutz
Thanks... yeah this worked fine.
@user69514 - More importantly, do you understand it?
Chris Lutz
+1  A: 

You didn't swap properly, that's why it didn't work.

#include <iostream>
#include <algorithm>

int const counter = 4;
char * title_arr[counter] = {
    "d", "c", "b", "a"
};

void sortArrays(){
    for(int i = 0; i < counter; i++){
        for(int j = 0; j < i; j++){
            if(strcmp(title_arr[i], title_arr[j]) < 0){
                char* title_temp = title_arr[i];
                title_arr[i] = title_arr[j];
                title_arr[j] = title_temp;
                //you wouldn't have made that stupid mistake this way.
                //std::swap(title_arr[i], title_arr[j]);
            }
        }
    }
}

int compare(void const * a, void const * b) {
    return strcmp(static_cast<char const *>(a), static_cast<char const *>(b));
}

struct StringLess : public std::binary_function<char const *, char const *, bool> {
    bool operator() (char const * a, char const * b) const {
        return strcmp(a, b) < 0;
    }
};

int main(int argc, char * argv[])
{
    sortArrays();
    //those ones better
//  qsort(title_arr, counter, sizeof(char *), compare);
//  std::sort(title_arr, title_arr + counter, StringLess());
    for (int i = 0; i < counter; i++) {
        std::cout << title_arr[i] << ", ";
    }
    return 0;
}
vava
+1  A: 

Bad coding style:
1. Don't use global variables. It's better to pass your array and length as arguments into sort function. Why? Your function is not reusable. What if you will need to sort another array? Yes, you will need to write another sort function...
2. More advanced tip: use emulation of higher-order function. What if you will need to sort not only characters? Integer, floats, strings or your own types. In this case you can also pass compare() function into your sort function which can compare objects of your array.