Creating a new array without duplicate integers from an array in C

Question

I was asked to create a new array without duplicate integars from an array.

I think I made a mistake in my code but can't notice anything wrong.

The output for "1, 2, 1, 2, 1, 4" is "1, 2, 2, 4".

It's supposed to be "1, 2, 4"

Would like to learn about my mistake.

    // Exercise 2 - 
void Ex2() {
    int i, counter = 1, size = -1;
    int* array = input_dynamic_array(&size);
    int* newArr = (int*)malloc((size)* sizeof(int));
    newArr[0] = array[0];
    assert(array);
    for (i = 1; i < size; i++) {
        if (!find_num_in_newArr(newArr, size, array[i])) {
            newArr[counter++] = array[i];
        }
    }
    newArr = (int*)realloc(newArr, (counter)*sizeof(int));
    printArray(newArr, counter);
    free(array);
    free(newArr);
}

bool find_num_in_newArr(int *newArr, int size, int num) {
    int i;
    for (i = 0; i < size; i++) {
        if (newArr[i] == num) {
            return true;
        }
        return false;
    }
}

int* input_dynamic_array(int *size)
{
    int *array;
    int ii;
    printf("Enter array size: ");
    scanf("%d", size);
    array = (int*)malloc((*size) * sizeof(int));
    assert(array);
    printf("Enter %d integer numbers: ", *size);
    for (ii = 0; ii < *size; ii++)
        scanf("%d", array + ii);
    return array;
}

the `for` loop in `find_num_in_newArr()` checks for `i < size-1`. Shouldnt it be `i < size` ? — Shanid, May 10 '18 at 11:44
Yes, noticed it and fixed it earlier, but still doesn't work, thanks — Tom S, May 10 '18 at 11:44

WedaPashi · Answer 1 · 2018-05-10T12:13:50.043

I see a problem here:

for (i = 0; i < size-1; i++) 
{
    if (newArr[i] == num) 
    {
        return true;
    }
    return false;
}

This will never have another iteration. It will either return true or false from the first iteration. Thats not what you are planning for when you used a loop.

Based on your design, you might want to move return false; outside the loop.

Another advise, Don't cast the return value of malloc, its pointless.

Also,

int* newArr = (int*)malloc((size)* sizeof(int));

This needs to be followed by a check. You need to check if malloc returned NULL. If yes, then memory is not allocated and anything with modification based on newArr would be horrible.

A relative cleaner way of doing it will include using a function that looks like below:

int RemoveDuplicates(int* Arr, int length)
{
    int i = 0, j = 0;
    int LengthChanged = 0;

    for (i = 1; i < length; i++)
    {
        for(j = 0; j < LengthChanged ; j++)
        {
            if(Arr[i] == Arr[j])
                break;
        }

        // Copy as is if there is not duplicate element in the array
        if (j == LengthChanged )
            Arr[LengthChanged++] = Arr[i];
    }

    return LengthChanged;
}

There is another problem in `Ex2` that will prevent correct operation. — chqrlie, May 10 '18 at 12:03
@chqrlie: Ah, I did see your answer covering that. To be honest I did't catch it, and it would be unfair to merely copy that up from your answer into mine. So I have already upvoted your answer, and its good that OP has marked yours as accepted. Thank you! — WedaPashi, May 14 '18 at 07:42

score 3 · Accepted Answer · answered May 10 '18 at 12:00

There are 2 problems in your code:

find_num_in_newArr exits the loop with a return false; at the first iteration. move the return statement outside the loop body.
Ex2 check the whole array for duplicates instead of just the portion already copied: find_num_in_newArr(newArr, size, array[i]) should be find_num_in_newArr(newArr, i, array[i]). As posted, this code has undefined behavior.

Here is a corrected version with a few extra assertions:

// Exercise 2 - 
bool find_num_in_newArr(const int *newArr, int size, int num) {
    int i;
    for (i = 0; i < size; i++) {
        if (newArr[i] == num) {
            return true;
        }
    }
    return false;
}

int *input_dynamic_array(int *size) {
    int *array;
    int ii;
    printf("Enter array size: ");
    assert(scanf("%d", size) == 1);
    assert(*size > 0);
    array = (int*)malloc((*size) * sizeof(int));
    assert(array != NULL);
    printf("Enter %d integer numbers: ", *size);
    for (ii = 0; ii < *size; ii++) {
        assert(scanf("%d", array + ii) == 1);
    }
    return array;
}

void Ex2(void) {
    int i, counter, size;
    int *array = input_dynamic_array(&size);
    int *newArr = (int*)malloc(size * sizeof(int));
    assert(newArr != NULL);
    newArr[0] = array[0];
    counter = 1;
    for (i = 1; i < size; i++) {
        if (!find_num_in_newArr(newArr, i, array[i])) {
            newArr[counter++] = array[i];
        }
    }
    newArr = (int*)realloc(newArr, counter * sizeof(int));
    assert(newArr != NULL);
    printArray(newArr, counter);
    free(array);
    free(newArr);
}

score 0 · Answer 3 · answered May 10 '18 at 11:50

You over complicate it a bit. Use the correct types for the indexes (size_t)

int CheckValue(int *arr, int value, size_t size)
{
    while (size--)
    {
        if (arr[size - 1] == value) return -1;
    }
    return 0;
}

int main()
{
    int arr[] = {1,4,5,8,3,2,1,-1,9,-1,-1,6,8,1,5,4,2,3};
    int newarr[sizeof(arr) / sizeof(arr[0])];
    size_t size = 0;

    for (size_t index = 0; index < sizeof(arr) / sizeof(arr[0]); index++)
    {
        if (!CheckValue(newarr, arr[index], size))
        {
            newarr[size++] = arr[index];
        }
    }
    return 0;
}

Creating a new array without duplicate integers from an array in C

3 Answers3