1

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;
}
5
  • 4
    change for(i=0; i<9; i++) to for(i=0; i<15; i++) Commented Jan 17, 2020 at 20:37
  • 1
    Have you done any debugging? Such as running your program in a debugger and/or adding more debug prints? This may help: How to debug small programs. Should be able to find the problem yourself with any basic debugging. Commented Jan 17, 2020 at 20:38
  • 1
    Why are you looping to 9 when there are 15 elements in the array? Commented Jan 17, 2020 at 20:39
  • 1
    "Does not work" is not a problem description. Please be specific (and consider reading about hash tables). Commented Jan 17, 2020 at 20:39
  • Instead of repeating the array size in multiple places, which makes it easy to get out of sync like this, define a macro: #define ARRAY_SIZE 15 and use that throughout the program. Commented Jan 17, 2020 at 20:40

1 Answer 1

3

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 
Sign up to request clarification or add additional context in comments.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.