How can I optimize this code which can get the length of non-repeated numbers in an array?
package com.sujay.practice;
import java.util.HashMap;
import java.util.Map;
/**
* Find the length of non-repeated numbers in an array
* @author Sujay
* 08-May-2018
*/
public class DuplicateValuesInArray {
public static void main(String[] args) {
int[] array = { 1, 2, 2, 3, 4, 5, 6, 2, 4, 5, 3 };
usingMap(array);
usingString(array);
}
private static void usingString(int[] array) {
String str = "";
for (int i = 0; i < array.length; i++) {
String temp = String.valueOf(array[i]);
if (!str.contains(temp)) {
str = str + temp;
}
}
System.out.println(str.length());
}
private static void usingMap(int[] array) {
Map<Integer, Integer> map = new HashMap<Integer, Integer>();
for (int i = 0; i < array.length; i++) {
map.putIfAbsent(array[i], i);
}
System.out.println(map.size());
}
}
-
1\$\begingroup\$ Welcome to Code Review! I hope you get some great answers. \$\endgroup\$Phrancis– Phrancis2018年05月08日 18:03:01 +00:00Commented May 8, 2018 at 18:03
2 Answers 2
The correct data structure to use to eliminate duplicates is a
Set
. TheString
processing is more expensive, and theMap
code is less clear because you don't care at all about the values.It's nice to provide a vararg parameter so that clients don't need to define an array if they don't want to.
It's a better practice to return your result from the method. If the client wants to print it, that's up to them. But if you print it, and the client changes what they want to do with it, they're out of luck.
You can used the enhanced
for
loop to make your code a little prettier.More descriptive names are better than less descriptive names.
If I were to take a stab at writing your code with the above changes in mind, it might look something like:
private static int countUniqueValues(final int... values) {
final Set<Integer> uniqueValues = new HashSet<>(values.length);
for (final int value : values) {
uniqueValues.add(value);
}
return uniqueValues.size();
}
Your first version is unreliable if there are numbers that consist of more than one digit. For example, the method usingString(int[])
would count only two distinct numbers in the array [1, 2, 12]
, and only one distinct number in the array [12, 1, 2]
.
Apart from that, you could also use a stream:
static int countDistinctIntegers(int[] array) {
return (int) Arrays.stream(array).distinct().count();
}
I think this better represents the intention behind the code than using a Map
(or a Set
, which, I agree, is more appropriate than a Map
here), because collecting the elements to a Collection
is not an aim per se, but just a means to accomplish something else, whereas the stream describes exactly what you want to do, seeing as streams are meant to represent operations on data as opposed to storage of data.