6
\$\begingroup\$

I've implemented a very simple resizable integer array in C, only including functionality to add and remove elements at various indexes.

I haven't looked at using void pointers to handle any type of data yet. I'm mainly wondering if my structure of the code is okay, with the header file containing external API details and the source file hiding away implementation detail.

vector.h

#ifndef VECTOR_H
#define VECTOR_H

typedef struct vector_struct* Vector;

extern Vector vector_create();
extern void vector_destroy(Vector vector);
extern void vector_add(Vector vector, int* element);
extern void vector_add_index(Vector vector, int* element, int index);
extern void vector_remove(Vector vector, int index);
extern void vector_pop(Vector vector);
extern void vector_print(Vector vector);

#endif

vector.c file:

#include "vector.h"
#include <stdlib.h>
#include <stdio.h>

struct vector_struct {
  int capacity;
  int number_of_elements;
  int* elements;
};

/*
Private function to allocate memory for vector creation
*/
static void* get_memory(size_t size) {
  void* memory;
  if ((memory = malloc(size)) == NULL) {
    exit(EXIT_FAILURE);
  }
  return memory;
}

/*
Private function to resize the amount of memory allocated for the given vector
*/
static void resize_vector(Vector vector, size_t size) {
  if ((vector->elements = realloc(vector->elements, size)) == NULL) {
    exit(EXIT_FAILURE);
  }
  vector->capacity = size / sizeof(int);
}

/*
Create and initialize a vector for use
*/
extern Vector vector_create() {
  Vector vector = get_memory(sizeof(Vector));

  vector->capacity = 10;
  vector->number_of_elements = 0;
  vector->elements = get_memory(sizeof(int) * 10);

  return vector;
}

/*
Free memory allocated for vector
*/
extern void vector_destroy(Vector vector) {
  free(vector->elements);
  free(vector);
}

/*
Add given element to the end of the vector
*/
extern void vector_add(Vector vector, int* element) {
  if (vector->number_of_elements+1 > vector->capacity) {
    resize_vector(vector, (vector->capacity * 2) * sizeof(int));
  }

  vector->elements[vector->number_of_elements] = *element;
  vector->number_of_elements++;
}

/*
Add given element at the given index in the vector
*/
extern void vector_add_index(Vector vector, int* element, int index) {
  if (index < 0 || index >= vector->number_of_elements) {
    exit(EXIT_FAILURE);
  }

  if (vector->number_of_elements+1 > vector->capacity) {
    resize_vector(vector, (vector->capacity * 2) * sizeof(int));
  }

  for(int i=vector->number_of_elements; i>index; i--) {
    vector->elements[i] = vector->elements[i-1];
  }

  vector->elements[index] = *element;
  vector->number_of_elements++;
}

/*
Removes element at given index, shifting all elements after that index
to the left once in order to fill in the empty index 
*/
extern void vector_remove(Vector vector, int index) {
  if (index < 0 || index >= vector->number_of_elements) {
    exit(EXIT_FAILURE);
  }

  for(int i=index; i<vector->number_of_elements-1; i++) {
    vector->elements[i] = vector->elements[i+1];
  }
  vector->number_of_elements--;

  if (vector->number_of_elements < (vector->capacity / 2)) {
    resize_vector(vector, (vector->capacity / 2) * sizeof(int));
  }
}

/*
Removes the last element in the vector
*/
extern void vector_pop(Vector vector) {
  if (vector->number_of_elements < 1) {
    exit(EXIT_FAILURE);
  }

  vector->number_of_elements--;
}

/*
Prints the vector's contents
*/
extern void vector_print(Vector vector) {
  printf("Capacity of vector: %d\nContents:\n", vector->capacity);
  printf("[");
  for (int i=0; i<vector->number_of_elements; i++) {
    printf((i == vector->number_of_elements-1) ? " %d " : " %d,", vector->elements[i]);
  }
  printf("]\n");
}
\$\endgroup\$
2
\$\begingroup\$
  • Do not typedef a pointer.

  • exit on a bad index is a bit too harsh. Consider returning a success/failure indication instead.

  • vector->capacity *= 2 and vector->capacity /= 2 embody the reallocation policy. It doesn't feel right that this policy is spread across many functions. I recommend to consolidate it in a single function (resize_vector is a good candidate), and call it unconditionally with the desired new size.

  • number_of_elements is not exposed to the client. It means that the client cannot iterate over the vector elements, which severely limits the utility of the array. Either expose it, or provide a mapping interface, along the lines of

    typedef (*op)(int, void *);
    .... map(Vector vector, op, int, void *);
    

    (the void * argument to share a state across the op invocations).

\$\endgroup\$
  • \$\begingroup\$ First point is refuted by the dupe of the post you linked to, at least for this case. \$\endgroup\$ – Deduplicator Jun 8 '18 at 22:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.