Dynamic array of integers

Question

I implemented a vector class in C for integers. Any feedback welcome.

header:

#define DEFAULT_CAPACITY 1000
// Define a vector type
typedef struct 
{
 int nrOfElements; // Current number of elements.
 int capacity; // Possible capacity. 
 int *data; // Pointer to array elements. 
} CVectorInt;
typedef enum {VECTOR_SUCCESS, VECTOR_OUTOFBOUNDS, VECTOR_MEMORYERROR} vector_error;
vector_error CVectorInit(CVectorInt *vector);
vector_error CVectorAppend(CVectorInt *vector, int value);
vector_error CVectorGet(CVectorInt *vector, int index, int * value);
vector_error CVectorSet(CVectorInt *vector, int index, int value);
void CVectorFree(CVectorInt *vector);

.c:

#include "CVectorInt.h"
#include <stdio.h>
#include <stdlib.h>
vector_error CVectorInit(CVectorInt *vector)
{
 int i = 0;
 vector->capacity = DEFAULT_CAPACITY;
 vector->nrOfElements = 0;
 vector->data = malloc(sizeof(int) * vector->capacity);
 if(vector->data == NULL)
 return VECTOR_MEMORYERROR;
 // Initialization in C never hurts.
 for(i = 0; i<DEFAULT_CAPACITY; i++)
 vector->data[i] = 0;
 return VECTOR_SUCCESS;
}
vector_error CVectorAppend(CVectorInt *vector, int value) 
{
 int * ptr = NULL;
 int i = 0;
 // Resize vector if there is no space for more elements.
 if(vector->capacity == vector->nrOfElements)
 {
 vector->capacity *= 2;
 ptr = realloc(vector->data, vector->capacity * sizeof(int));
 if(ptr == NULL) 
 { 
 return VECTOR_MEMORYERROR;
 }
 else
 {
 vector->data = ptr; 
 // Initialize new elements to 0.
 for(i = vector->nrOfElements; i<vector->capacity; i++)
 vector->data[i] = 0;
 }
 }
 vector->data[vector->nrOfElements++] = value;
 return VECTOR_SUCCESS;
}
vector_error CVectorGet(CVectorInt *vector, int index, int * value)
{
 if(index < vector->nrOfElements && index >= 0)
 {
 // Pass value at given index.
 *value = vector->data[index];
 return VECTOR_SUCCESS;
 }else
 {
 return VECTOR_OUTOFBOUNDS;
 }
}
vector_error CVectorSet(CVectorInt *vector, int index, int value)
{
 if(index >= vector->capacity)
 return VECTOR_OUTOFBOUNDS;
 vector->data[index] = value;
 return VECTOR_SUCCESS;
}
void CVectorFree(CVectorInt *vector)
{
 free(vector->data);
}

Usage:

#include <stdio.h>
#include <stdlib.h>
#include "CVectorInt.h"
int main(void) 
{
 //error handling omitted for simplicity
 CVectorInt v;
 int i = 0, temp = 0;
 CVectorInit(&v);
 for(i = 0; i<100; i++)
 {
 CVectorAppend(&v, i*2);
 }
 for(i = 0; i<100; i++)
 {
 CVectorGet(&v, i, &temp);
 printf("%d ", temp);
 }
 CVectorFree(&v);
 return 0;
}

• Pimgd: Have you handled overflow errors appropriately?
• user100282: @Pimgd what kind of overflow errors are you referring to
• Mast: I have rolled back the last edit. Please see What to do when someone answers .
• Pimgd: @User100 I recommend you go with a follow up question, you got answers here, apply the changes, apply your own changes, as a new question, mark an answer here as accepted and ignore this one.
• Mast: @User100 If you disagree with any decision a user made, take it to Code Review Meta or find us in The 2nd Monitor.

Answer 1

I think you should use uint for things where you cannot have negative values.

vector_error CVectorGet(CVectorInt *vector, int index, int * value)
{
 if(index < vector->nrOfElements && index >= 0)

This check, for instance, wouldn't need the comparison with 0 if you used uint.

In the comments I already said something about overflow. Now, you can indeed say "that's such a big amount of elements, people are not going to surpass this" and not build it in. That's okay. Be sure to add it in as a comment in the header though, because you don't want to accidentally find out that this is a problem.

• Marco13: Regarding the unsigned: Also see stackoverflow.com/q/30395205

Answer 2

Some points:

• CVectorSet: what should happen if index > vector->nrOfElements? Should nrOfElements=index?
• The vector_error enum should be rename as vector_status, I think.
• vector->capacity *= 2; This is a geometric progression, it may grows faster then the user think. You may want to use an arithmetic progression (vector->capacity += DEFAULT_CAPACITY; for example).
• vector->capacity *= 2; You should fix a limit to the capacity.
• if(index >= vector->capacity) return VECTOR_OUTOFBOUNDS; Why not checking the lower bound too? (Or use unsigned int.)
• Since you're initializing the space to zero, you should use calloc instead of malloc.

• user100282: "Should nrOfElements=index?" no I just increase nrOfElements
• user100282: if nrOfElements>index then I don't increase nrOfElements