Search code examples
arrayscalgorithmfunctionresizable

Append function not working in resized array


I'm building an array that increments its size when it's totally full.

The situation is, the user calls append with value of 5, but the array (with 4 slots) is full. My program calls my resize function, which turns the array into 8 slots maintaining the 4 old values.

The resize implementation is working, but the part where it appends the new value, the slot specified continues empty. Like this:

Initial array:
[1, 2, 3, 4]
Array after resize and append:
[1, 2, 3, 4, -1, -1, -1, -1]
Expected result:
[1, 2, 3, 4, 5, -1, -1, -1]
#define EMPTY -1
size_t capacity = 4;
int size = 0;

int main(void)
{
    int *arr = malloc(capacity * sizeof(int));
    arr[0] = 1;
    arr[1] = 2;
    arr[2] = 3;
    arr[3] = 4;
    size = 4;

    appendInTheEnd(arr, 5, &arr);
    for (int i = 0; i < capacity; i++)
    {
        printf("%d ", arr[i]);
    }
}


void appendInTheEnd(int *arr, int value, int **array)
{
    if (size == capacity)
    {
        resizeArray(array, capacity);
        capacity *= 2;
        arr[size] = value;
        size++;
    }
}

void resizeArray(int **arr, size_t capacity)
{
    int *newArr = malloc(2 * capacity * sizeof(int));
    for (int i = capacity; i < 2 * capacity; i++)
    {
        newArr[i] = EMPTY;
    }
    memcpy(newArr, *arr, capacity * sizeof(int));
    memset(newArr + capacity, EMPTY, capacity);
    free(*arr);
    *arr = newArr;
}


Solution

  • appendInTheEnd() doesn't do anything unless size == capacity. Don't pass in both arr and array. You just need **arr. Here is a minimal change to fix the issue (see @0___________'s answer for a bigger step):

    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    
    #define EMPTY -1
    size_t capacity = 4;
    int size = 0;
    
    void resizeArray(int **arr, size_t capacity) {
        int *newArr = malloc(2 * capacity * sizeof(int));
        for (int i = capacity; i < 2 * capacity; i++)
        {
            newArr[i] = EMPTY;
        }
        memcpy(newArr, *arr, capacity * sizeof(int));
        memset(newArr + capacity, EMPTY, capacity * sizeof(int));
        free(*arr);
        *arr = newArr;
    }
    
    void appendInTheEnd(int **arr, int value) {
        if(size == capacity) {
            resizeArray(arr, capacity);
            capacity *= 2;
        }
        (*arr)[size++] = value;
    }
    
    
    int main(void) {
        int *arr = malloc(capacity * sizeof(int));
        arr[0] = 1;
        arr[1] = 2;
        arr[2] = 3;
        arr[3] = 4;
        size = 4;
    
        appendInTheEnd(&arr, 5);
        for (int i = 0; i < capacity; i++) {
            printf("%d ", arr[i]);
        }
    }
    

    and it will print:

    1 2 3 4 5 -1 -1 -1
    

    I would change the print loop to iterate till size instead of capacity. It's perfectly ok to leave the values >= size uninitialized (no need for memset in resizeArray(); note that in memset it should be capacity * sizeof(int) not just capacity; I had a mistake in previous answer since fixed),

    The next step with your program, btw, is to make a struct to hold your state so it's not global values:

    struct int_array {
       int *data;
       size_t size;
       size_t capacity;
    };
    

    Then you write a int_array_init() function that initialize your struct, and you change the signature of resizeArray() and appendInTheEnd to take a pointer to struct *int_array; along with whatever else information is needed. It's a good idea to then rename your functions a prefix of your struct so:

    appendInTheEnd() renamed to int_array_append()
    resizeArray() renamed to int_array_resize()