Как реализовать сортировку в пределах вектора? [закрыто]

Работает следующее:

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

typedef unsigned int uint;

#define GOOD 0
#define BAD  ( assert(0), -1 )

struct SVec
{
    int  *pData;
    uint uiCount;
};

void VecShow(struct SVec const * const vec);
int VecAdd(struct SVec * const vec, int const iVal);
void VecSwap(struct SVec * vec, int const iVal, int const jVal);
int VecSet(struct SVec * const vec, int const index, int const iVal);
int VecIndexAt(struct SVec const * const vec, int const iVal);
int VecGet(struct SVec const * const vec, int const index);
int VecContains(struct SVec const * const vec, int const iVal);


int VecAdd(struct SVec * const vec, int const iVal) {
    int newsz = vec->uiCount + 1;
    if (vec->pData == 0)
        vec->pData = (int*)malloc(newsz * sizeof(int));
    else
        vec->pData = (int*)realloc(vec->pData, newsz * sizeof(int));

    *(vec->pData + vec->uiCount) = iVal;
    vec->uiCount++;

    return 1;
}

void VecShow(struct SVec const * const vec) {
    int count = vec->uiCount;
    int index;

    if (vec->uiCount == 0) return;

    for (index = 0; index <= count; index++) {
        if (index == count - 1) {
            printf("%d", *(vec->pData + index));
            break;

        }
        printf("%d, ", *(vec->pData + index));
    }

    printf("\n");
}


void VecSwap(struct SVec * vec, int const iVal, int const jVal) {
    int temp = VecGet(vec, iVal);
    int iValIndex = VecIndexAt(vec, iVal);
    int jValIndex = VecIndexAt(vec, jVal);
    VecSet(vec, iValIndex, iVal);
    VecSet(vec, jValIndex, temp);

}

int VecSet(struct SVec * const vec, int const index, int const iVal) {
    if (!VecContains(vec, iVal) || index > vec->uiCount || index < 0) return BAD;
    vec->pData[index] = iVal;
}

int VecIndexAt(struct SVec const * const vec, int const iVal) {
    int index;
    if (!VecContains(vec, iVal))
        return BAD; //value wasn't found
    for (index = 0; index < vec->uiCount; index++) {
        if (*(vec->pData + index) == iVal)
            return index;
    }

    return BAD; //value wasn't found, should not be able to get here but let's be explicit
}

int VecContains(struct SVec const * const vec, int const iVal) {
    int index;
    if (iVal < 0) return BAD;
    for (index = 0; index < vec->uiCount; index++) {
        if (*(vec->pData + index) == iVal) {
            return GOOD;
        }
    }
    return BAD;
}

int VecGet(struct SVec const * const vec, int const index) {
    if (index > vec->uiCount || vec->uiCount <= 0 || index < 0)
        return BAD; //failure
    return vec->pData[index];
}

void VecSort(struct SVec * vec) {
    int i;
    int j;
    for (i = 0; i < vec->uiCount - 1; i++) {
        for (j = 0; j < vec->uiCount - i - 1; j++) {
            if (VecGet(vec, j) > VecGet(vec, j + 1)) {
                int tmp = vec->pData[j];
                vec->pData[j] = vec->pData[j + 1];
                vec->pData[j + 1] = tmp;
            }
        }
    }
}


int main()
{
    struct SVec vec = {0};
    for (size_t i = 5; i > 0; i--) {
        VecAdd(&vec, i);
    }
    VecShow(&vec);
    VecSort(&vec);
    VecShow(&vec);
    // leak memory
    return 0;
}

выводит:

5, 4, 3, 2, 1                                                                                                              
1, 2, 3, 4, 5      

1. В своем коде вы неправильно используете адрес оператора &, когда применяете его к указателям. Например, в void VecSwap(struct SVec * vec, int const iVal, int const jVal) { int temp = VecGet(&vec, iVal); - &vec - это адрес указателя struct SVec * vec (не данных!), Поэтому вы передаете функции VecGet указатель struct SVec **, т.е. указатель на указатель на данные. Это делает код недействительным в нескольких местах. Компилятор не должен хотя бы предупреждать вас об этом.
2. В вашей функции сортировки после сравнения элементов по индексам j и j + 1 вы хотите поменять местами элементы по этим индексам. Не элементы, которые хранят значение j (которое может быть с любым индексом или вообще без него), а элементы точно с индексами j и j + 1.
3. Это не имеет смысла в return BAD, если #define BAD -1. Как я собираюсь хранить минус один в массиве? Является ли -1 значительным числом? Обычно программисты выбирают максимальное (INT_MAX) или минимальное значение (INT_MIN) для возврата ошибки или возвращают 0, а также устанавливают глобальный флаг (глядя на вас strtol ...)
4. [1149 ] Часть:

   if (vec->pData == 0)
       vec->pData = (int*)malloc(newsz * sizeof(int));
   else
       vec->pData = (int*)realloc(vec->pData, newsz * sizeof(int));
   

   можно упростить до:

   vec->pData = realloc(vec->pData, newsz * sizeof(int));
   

   realloc(NULL, ...) равно вызову malloc(...). И 0 неявно преобразуется в NULL, поскольку NULL определяется как (void*)0 (давайте не будем останавливаться на этом подробнее ...)

5. Я видел большую часть из *(arr + index) ] синтаксис на протяжении многих лет, но я не думаю, что здесь есть какое-либо оправдание. Вы индексируете массив, просто arr[index], который в точности эквивалентен (и, пожалуйста, не index[arr])
6. const struct SVec * выглядит лучше и более распространенным для меня, чем struct SVec const *.
7. Как отмечает @Eric - функция void VecShow(...) содержит return -1, по крайней мере, неопределенное поведение, и я был удивлен, обнаружив, что она на самом деле скомпилирована в архаичной версии gcc, которую я использовал.

8. Было бы неплохо включить проверку ошибок распределения. Обычно при использовании realloc правильным способом является использование временного указателя.

   void * const tmp = realloc(vec->pData, newsz * sizeof(*vec->pData));
   if (tmp == NULL) {
       // the old pointer vec->pData is still valid!
       return BAD;
    }
    // success
    vec->pData = tmp;
   

9. За исключением этих ошибок, это выглядит как очень хороший код с хорошей инкапсуляцией и организацией. Хорошая структура, есть при необходимости указатели const, хорошие отступы, много проверок, очень удобочитаемые, хорошие. И извините за мой еще не идеальный английский.