11
\$\begingroup\$

Here is my code for removing duplicated values from an array. I think I tested it with the most possible cases. Any suggestions or bugs?

class duplicate {

    public static int[] removeDuplicates(int[] arr) { 
        int end = arr.length;

        for (int i = 0; i < end; i++) {
            for (int j = i + 1; j < end; j++) {
                if (arr[i] == arr[j]) {                  
                    int shiftLeft = j;

                    for(int k = j + 1; k < end; k++, shiftLeft++) {
                        arr[shiftLeft] = arr[k];
                    }

                    end--;
                    j--;
                }
            }
        }

        int[] whitelist = new int[end];

        for (int i = 0; i < end; i++) {
            whitelist[i] = arr[i];
        }

        return whitelist;
    }
}

After some tests, it appears really inefficient because an array with 1,000,000 elements takes a very long time to end. Is there any better way to implement this on arrays?

\$\endgroup\$
1
  • 2
    \$\begingroup\$ Use Set(Eg HashSet). Duplicate values are not allowed in it. Just iterate over the array and add it to the Set.If you want result in an array copy the set to the target array. \$\endgroup\$ Commented Jul 31, 2013 at 9:59

3 Answers 3

10
\$\begingroup\$

You're following the same philosophy as the bubble sort, which is very, very, very slow. Have you tried this?:

  • Sort your unordered array with quicksort. Quicksort is much faster than bubble sort (I know, you are not sorting, but the algorithm you follow is almost the same as bubble sort to traverse the array).
  • Then start removing duplicates (repeated values will be next to each other). In a for loop you could have two indices: source and destination. (On each loop you copy source to destination unless they are the same, and increment both by 1). Every time you find a duplicate you increment source (and don't perform the copy).
\$\endgroup\$
0
11
\$\begingroup\$

Suggestion:

public static Integer[] removeDuplicates(Integer[] arr) {
  return new HashSet<Integer>(Arrays.asList(arr)).toArray(new Integer[0]);
}

Another solution might be:

public static int[] removeDuplicates(int[] arr) {
  Set<Integer> alreadyPresent = new HashSet<Integer>();
  int[] whitelist = new int[0];

  for (int nextElem : arr) {
    if (!alreadyPresent.contains(nextElem)) {
      whitelist = Arrays.copyOf(whitelist, whitelist.length + 1);
      whitelist[whitelist.length - 1] = nextElem;
      alreadyPresent.add(nextElem);
    }
  }

  return whitelist;
}

Here you only iterate once via arr.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ You could avoid copying the whitelist for every unique element, and start it off by having the same size as the original, then only copy the subarray with unique lements just before returning. \$\endgroup\$
    – bowmore
    Commented Jul 31, 2013 at 11:26
  • 1
    \$\begingroup\$ Incrementally increasing whitelist by 1 creates a time complexity of O(N^2). Consider using ArrayList which expands dynamically, and returning arraylist.toArray(); That would run in O(N). \$\endgroup\$ Commented Aug 10, 2013 at 19:47
  • \$\begingroup\$ @awashburn if targeting >= .NET 2.0, one should use List<T> instead of ArrayList. \$\endgroup\$ Commented Nov 6, 2013 at 3:06
10
\$\begingroup\$

A slight improvement on Dr H's

public static int[] removeDuplicates(int[] arr) {
    Set<Integer> alreadyPresent = new HashSet<>();
    int[] whitelist = new int[arr.length];
    int i = 0;

    for (int element : arr) {
        if (alreadyPresent.add(element)) {
            whitelist[i++] = element;
        }
    }

    return Arrays.copyOf(whitelist, i);
}

This does only one array copy at the end. It also takes advantage of the fact that Set.add() returns a boolean that indicated if the Set changed, and so avoids an explicit contains() check.

Java 8 update

In java 8 the code is a lot simpler :

public static int[] removeDuplicates(int[] arr) {
    return Arrays.stream(arr)
            .distinct()
            .toArray();
}
\$\endgroup\$
2
  • \$\begingroup\$ why you are copying whitelist ? Can't we just return whitelist ? \$\endgroup\$ Commented Sep 11, 2015 at 11:55
  • \$\begingroup\$ @PrabhatSubedi copy because the size will (most likely) be smaller than the original. And whitelist has the original size. \$\endgroup\$
    – bowmore
    Commented Sep 11, 2015 at 14:07

Not the answer you're looking for? Browse other questions tagged or ask your own question.