-2

I want to find the common elements between two arrays with different sizes and put them in another array. Can you tell what's wrong with my code?

    public static int[] numratEQelluar(int[] vargu, int[]varguPer)
{
    int count = 0;
    int [] nrQelluar = new int[count];
    for(int i = 0; i<vargu.length; i++)
    {
        for(int idx = 1;idx<varguPer.length ; idx++)
        {
            if(vargu[i] == (varguPer[idx]))
            {
                count++;
                for(int index = 0; index<nrQelluar.length; index++)
                {
                    nrQelluar[index] = vargu[i];    
                }

            }
        }

    }
    return nrQelluar;
user1803551
  • 12,965
  • 5
  • 47
  • 74
  • 1
    Start by editing the tags of your question to include what language your code is written is, at the moment it is too broad, since the code may be valid at multiple languages – Ferrybig Jan 25 '16 at 13:39
  • @Ferrybig, sorry I forget it. The code is in Java. – Edon Gjergji Jan 25 '16 at 13:45
  • 1
    you never update the length of nrQelluar. You start it with size 0 and it ends with size zero. Maybe you should start it with the length of the shorter array. – matt Jan 25 '16 at 13:46
  • @matt is correct. You `nrQelluar` is always length zero. You should start by making the arrays the same size and then you can shorten the length later once you know how many elements it should have. – GrizzlyManBear Jan 25 '16 at 13:48
  • You can also have a look at following: http://stackoverflow.com/questions/17863319/java-find-intersection-of-two-arrays – user2004685 Jan 25 '16 at 14:08
  • In addition to the size of nrQuellular, you never look at the first element of varguPer and whenever you find a match, you'll overwrite all of nrQuellular with that – Simon Jan 25 '16 at 14:13

2 Answers2

0

The reason it doesn't work is indeed due to incorrect memory allocation to nrEqualler as been said in the comments.

However, there are a few thing i'd change in your code:

  1. using a LinkedList instead of primitive int [] for dynamic sized array is much more efficient (add in O(1)).

  2. less indentations and confusing indexes by extracting methods.

So this should do:

public static int[] numratEQelluar(int[] vargu, int[]varguPer){
    List<Integer> nrQelluar = new LinkedList<Integer> ();
    for(int i = 0; i < vargu.length; i++) {
        if (contains(varguPer, vargu[i])) nrQelluar.add(vargu[i]);
    }
    return toPrimitive(nrQelluar);
}

private static boolean contains(int [] array, int num){
    for(int i = 0; i < array.length; i++){
        if(array[i] == num) return true;
    }
    return false;
}

private static int[] toPrimitive(List<Integer> list) {
    int[] primitiveArray = new int[list.size()];
    for(int i = 0; i < list.size(); i++){
        primitiveArray[i] = list.get(i);
    }
    return primitiveArray;
}
Daniel
  • 6,194
  • 7
  • 33
  • 59
0

The problem is between these lines of code

int count = 0;
 int [] nrQelluar = new int[count];

The size of your new array will be zero. try changing this to the sum of the length of both the arrays.

Updated with a working code.

Try this code :

public static Integer[] findCommonElements(int[] arr1, int[] arr2){
    HashSet set = new HashSet<>();
    for(int i=0;i<arr1.length;i++){
      for(int j=0;j<arr2.length;j++){
        if(arr1[i] == arr2[j]){
          set.add(arr1[i]);
        }
      }
    }
    Integer[] resultArray = (Integer[]) set.toArray(new Integer[set.size()]);
    return resultArray;
  }

Using a set will ensure that you won't get any duplicates in the resulting array. If duplicates are fine, then you can use ArrayList to store the common elements and then convert them into Array.

thedarkpassenger
  • 7,158
  • 3
  • 37
  • 61
  • I want to make the length of array exactly how many number are the same. For ex. If are 4 common numbers I want the array length to be 4. – Edon Gjergji Jan 25 '16 at 16:14