print common elements from two arrays

Question

I'm trying to find common elements from two unsorted arrays with same size....

I couldn't get expected elements as a output. whats's wrong with my code.

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

int f=0, k=0;
int arr[10];
int main()
{
    int *a,*a1,n,i,c;

    printf("enter no of elements in arrays  ");
    scanf("%d", &n);

    a = (int *)malloc(sizeof(int *)*n);
    a1 = (int *)malloc(sizeof(int *)*n);


    printf("enter elements 1st array\n");
    for(i=0; i<n; i++)
    {
        scanf("%d", &a[i]);
    }

    printf("enter elements 2nd array\n");   
    for(i=1; i<=n; i++)
    {
        scanf("%d", &a1[i]);
    }

    commonelefind(a,a1,n);

    if(k == 0)
    {
        printf("there is no common element");
    }
    else
    {
        printf("common elements are");
        while(k!=0)
        {
            printf("%d\n", arr[k]);
            k--;
        }
    } 
}

commonelefind(int *a, int *a1, int n)
{
    int i,j,ele;

    for(i=1;i<=n;i++)
    {
        ele = a[i];
        for(j=1;j<=n;j++)
        {
            if (ele == a1[j])
            {
                k++;
                arr[k] = ele;
            }
        }
    }
    return *arr,k;
}

Answer 1

Your program doesn't compile nor run, so "couldn't get expected elements" seems an understatement. Let's fix up your code so that it compiles and in the process, eliminate the global variables which aren't a good idea:

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

/* we can't return two things so the result array length is returned
 via the arguments (unfortunately) */
int *commonelefind(int *array1, int *array2, int source_size, int *result_size)
{
    int i, j;

    /* result array will always be less than or equal to sum of source arrays */
    int *result = (int *)malloc(sizeof(int) * source_size * 2);

    *result_size = 0;

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

        for (j = 0; j < source_size; j++)
        { 
            if (element == array2[j])
            { 
                result[*result_size] = element;

                *result_size += 1;
            }
        }
    }

    return result;
 }


int main()
{
    int *a1, *a2, i, n, *common, length;

    printf("enter no of elements in arrays: ");
    scanf("%d", &n);

    a1 = (int *)malloc(sizeof(int) * n);
    a2 = (int *)malloc(sizeof(int) * n);

    printf("enter elements 1st array\n");

    for (i = 0; i < n; i++)
    {
        scanf("%d", &a1[i]);
    }

    printf("enter elements 2nd array\n");   

    for (i = 0; i < n; i++)
    {
        scanf("%d", &a2[i]);
    }

    common = commonelefind(a1, a2, n, &length);

    if (length == 0)
    {
        printf("there is no common element");
    }
    else
    {
        printf("common elements are:\n");

        for (i = 0; i < length; i++)
        {
            printf("%d\n", common[i]);
        }
    }

    free(common);  /* we're obligated to free this when we're finished */

    return 0;
}

Answer 2

There are sevelar errors in your program. Some have already been addressed by commenters.

In C, array indices start at 0. Ranges of indices are represented with an inclusive lower bound and an exclusive upper bound. If you have an array of n elements, n is not a valid index; it is one beyond the valid range. The index of the last element is n - 1.

This convention means that the typical for lopp look like this:

for (int i = 0; i < n; i++) ...

Seeing a start index of 1 or an less-than-or-equal sign in the condition should make you wary. (Such constructs need not be false necessarily, but when you stick to the usual C nmomenclature, the are unusual.)

That also means that when you walk through an array backwards and start at the array length, n, you must decrement your index before using it, because n itself isn't a valid index. Your loop to print the elements of arr backwards should therefore be:

    while (k != 0) printf("%d\n", arr[--k]);

or, maybe better:

    while (k-- > 0) printf("%d\n", arr[k]);

(The latter varient is probably better, because here, k` hold a valid index throughout the loop body. It doesn't matter here, where the loop body is just one statement, but is preferrable for larger loops.)

The same logic goes for appending elements to an array: You use the old index to access the element and then increment to the new length:

    arr[k++] = a[i];

because (yadda, yadda) the new length k is out of the bounds of the (currently valid) array range.

You call the function, before you define it. That means that the compiler doens#T know its signature and derives one from the used arguments. This is not a good practice. Declare a function prototype before you use it:

void commonelefind(int *a, int *a1, int n);

I've made the function void, because you don't really return anything; the result is stored in global ariables. This is not a good design, but it works for small programs. Your return statement:

return *arr, k;

is probably not what you want: It returns k, the right-hand side of the comma operator. *arr is evaluated, but without effect. Don't return anything here.

Lastly, be more meticulous with how you print things: There should be a new-line character at the end of your strings, so that results and messages don't run together.

Here's a corrected variant with hard-coded arrays. It still considers elements multiple times if they are present more than once in one array. (This makes the dimension of arr too small, because in the worst case of two arrays consisting of the same repeated element you have n*n hits), but I leave that as an exercise:

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

void commonelefind(int *a, int *a1, int n);

int f = 0, k = 0;
int arr[10];

int main()
{
    int a[] = {3, 4, 1, 6, 7, 8, 3, 4, 5};
    int b[] = {2, 4, 6, 8, 10, 12, 14, 16, 18};
    int n = 9;

    commonelefind(a, b, n);

    if (k == 0) {
        printf("there is no common element\n");
    } else {
        printf("common elements are\n");
        while (k-- > 0) {
            printf("%d\n", arr[k]);
        }

    }

    return 0;
}

void commonelefind(int *a, int *b, int n)
{
    int i, j;

    k = 0;
    for (i = 0; i < n; i++) {
        for (j = 0; j < n; j++) {
            if (a[i] == b[j]) {
                arr[k++] = a[i];
            }
        }
    }
}