Search code examples
carrayscopyunique

Copying an unsorted C array into another while avoiding duplicates


Hello everyone and thanks for your time.

For exercise, I wanted to write a program which copies all elements from an array to another array but without the duplicates. The only condition is that I cannot change the original array - so no sorting.

I tried making a function which checks if the current element of array1 is found in the array we copy to (array2). If no, we then copy the element to the second array and increase the size by one.

However, it does not work:

If I have

int array1[15] = {3,2,4,7,9,1,4,6,7,0,1,2,3,4,5};
int array2[15];

array2 should contain the following numbers: 3,2,4,7,9,1,6,0,5

But my output is as follows: 3,2,4,7,9,1,6

Here is the code:

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

int already_exists(int array2[], int size_arr2, int element)
{

    int i;

    for(i=0; i<size_arr2; i++)
    {
        if(array2[i] == element)
            return 1;
    }
    return 0;
}

int main()
{
    int array1[15] = {3,2,4,7,9,1,4,6,7,0,1,2,3,4,5};
    int array2[15];
    int i;
    int size_arr2=0;

    for(i=0; i<9; i++)
    {
        int element = array1[i];

        if(already_exists(array2, size_arr2, element) == 1)
            continue;
        else
        {
            array2[size_arr2] = element;
            size_arr2++;
        }
    }

    for(i=0; i<size_arr2; i++)
    {
        printf("%d, ", array2[i]);
    }
    return 0;
}

Solution

  • You have a typo in the for loop

    for(i=0; i<9; i++)
    

    The array array1 contains 15 elements. So the loop should look like

    for ( i = 0; i < 15; i++ )
    

    The reason of the error is that you are using "magic numbers" instead of named constants.

    Nevertheless the program in whole is inefficient because the function already_exists is called for each element of the array array1. At least you could declare it as an inline function.

    Moreover it should be declared like

    int already_exists( const int array2[], size_t size_arr2, int element );
    

    Instead of this function it is better to write a function that performs the full operation.

    Here is a demonstrative program.

    #include <stdio.h>
    
    size_t copy_unique( const int a1[], size_t n1, int a2[] )
    {
        size_t n2 = 0;
    
        for ( size_t i = 0; i < n1; i++ )
        {
            size_t j = 0;
            while ( j < n2 && a2[j] != a1[i] ) j++;
    
            if ( j == n2 ) a2[n2++] = a1[i];
        }
    
        return n2;
    }
    
    int main(void) 
    {
        enum { N = 15 };
        int array1[N] = { 3, 2, 4, 7, 9, 1, 4, 6, 7, 0, 1, 2, 3, 4, 5 };
        int array2[N];
    
        for ( size_t i = 0; i < N; i++ ) printf( "%d ", array1[i] );
        putchar( '\n' );
    
        size_t n2 = copy_unique( array1, N, array2 );
    
        for ( size_t i = 0; i < n2; i++ ) printf( "%d ", array2[i] );
        putchar( '\n' );
    
        return 0;
    }
    

    Its output is

    3 2 4 7 9 1 4 6 7 0 1 2 3 4 5 
    3 2 4 7 9 1 6 0 5