Return to Answer

• 1

• 36.2k
• 2
• 43
• 96

Correctness

1. Good use of size_t.

2. Good use of const with various functionals like gcl_vector_empty(const gcl_vector *v)

3. Something of a correctness issue. Better clean-up and rather see symmetry with the "first" and "last" functions. I want incorrect re-usage of a free'd variable to cause problems as quickly as possible. Also like free(), be tolerant of a NULL argument. Also: consider complementary function names rather than init and destroy.

    gcl_vector *gcl_vector_init(void) { ... }
    gcl_vector *gcl_vector_destroy(gcl_vector *v) {
    if (v) {
    free(v->data);
    v->data = NULL;
    v->size = 0;
    free(v);
    }
    return NULL;
    }
   

4. Watch out for 0-1 which is SIZE_MAX. Anywhere - occurs is a potential error.

    GCLError gcl_vector_set(gcl_vector *v, void *elem, size_t pos) {
    // v->size could be 0 here.
    // if (pos > v->size - 1 || ...
    if (pos + 1 > v->size || ...
   

5. BTW: pos + 1 > v->size is a sufficient check. No need for || gcl_vector_empty(v)

    if (pos + 1 > v->size) { // out of range
   

6. Pedantic: MAX_VECTOR_SIZE not enforced with v->capacity * 2

    const size_t newElemCount = v->capacity == 0 ? INITIAL_VECTOR_CAPACITY : v->capacity * 2;
   

Performance Considerations (both memory and time improvements)

1. Good that the true initial vector size is 0. A popular usage of this type could easily have numerous zero-length vectors. Disagree with INITIAL_VECTOR_CAPACITY = 100 as 100 seems a bit high, but that is a quibble.

Code Style

1. In the header files, sometimes the user does not has or does not care for access to the .c source code. Conveying information in the header is useful. Recommend using variable names

    // typedef int (*GCLCompare)(const void *, const void *);
    typedef int (*GCLCompare)(const void *element_x, const void *element_y);
   

2. With enumeration, Found it useful to create a N value to help check the range.

    typedef enum GCLError {
    eNoErr = 0,
    eInvalidArg,
    ...
    eOutOfBoundsAccess,
    GCLError_N
    } GCLError;
    if (error < 0 || error >= GCLError_N) Handle_BadError(error);
   

3. Minor: Prefer explicit void. Need to double check if () is equivalent. I think, as a declaration, foo() and foo(void) are equivalent but not as a lone definition.

    // gcl_vector *gcl_vector_init();
    gcl_vector *gcl_vector_init(void);
   

etc.

1. void gcl_vector_clear(gcl_vector *v) is a good function. Something useful for the user.

2. Consider a compare function that pass the context - re

    errno_t qsort_s(void *base, rsize_t nmemb, rsize_t size, 
    int (*compar)(const void *x, const void *y, void *context), 
    void *context);`
   

3. I think gcl_vector_remove() should return a success/fail indication.

4. A small sample usage, as a #if 0 ... #endif comment in the cgcl_vector.h file goes a long way to helping folks understand the basics of this tool set.

5. As mentioned by @vnp, A useful "right size" function is missing.

6. Maybe a complementary function: bool gcl_vector_full(const gcl_vector *v); to go with bool gcl_vector_empty();