patternjavaModerate
Optimize random number generator
Viewed 0 times
randomnumberoptimizegenerator
Problem
I have a method that generates a 10 digit random number where each digit occurs only once.
eg:
Is there any way this method could be optimized/improved?
eg:
0123456789
3645720918Is there any way this method could be optimized/improved?
private static String randomize(){
Random r = new Random();
List digits= new ArrayList();
String number = "";
for (int i = 0; i 0; i--) {
int randomDigit = r.nextInt(i);
number+=digits.get(randomDigit);
digits.remove(randomDigit);
}
return number;
}Solution
If you are driven to get the absolute best performance, there are a number of things I would change. These changes will not necessarily improve the readability, but the performance will be best.
First up, creating a
The second thing is "Why use a List?". Seriously, it's overkill. Use a primitive array, and preferably use char.....
String concatenation is slow, so the
If you insist on using String concatenation then you would be better off using:
Finally, when there is just one member left in the array there is no need to do the random lookup (it will always return 0), so you can just use
Taking the above things in to consideration, I would recommend the following (which has a couple of other differences that I 'like'):
EDIT
I just realized another tweak could squeeze a little more performance out too.... consider this implementation which does not even need to create an array on each invocation, and who cares what order the digits are when you are just going to reshuffle them anyway. Thus, this saves creating a
First up, creating a
new Random() each time is going to slow you down. This may well be about half of your time, in fact. I recommend a static version if you can be sure you won't have threading issues, or alternatively I would recommend a ThreadLocal. In general a ThreadLocal is the better option because even though Random is thread-safe, you don't want contention when it locks one thread against another.The second thing is "Why use a List?". Seriously, it's overkill. Use a primitive array, and preferably use char.....
String concatenation is slow, so the
number += digits.get(randomDigit); is essentially the same as:StringBuilder sb = new StringBuilder(number);
sb.append(String.valueOf(digits.get(randomDigit));
number = sb.toString();If you insist on using String concatenation then you would be better off using:
StringBuilder sb = new StringBuilder(10);
for (...) {
...
sb.append(digits.get(randomDigit));
...
}
return sb.toString();Finally, when there is just one member left in the array there is no need to do the random lookup (it will always return 0), so you can just use
1 as the limit of your loop and simply append the last remaining Integer from your List.Taking the above things in to consideration, I would recommend the following (which has a couple of other differences that I 'like'):
private static final ThreadLocal RANDOM = new ThreadLocal() {
public Random initialValue() {
return new Random();
}
}
public static final String randomize() {
final char[] digits = "0123456789".toCharArray();
final Random rand = RANDOM.get();
int index = digits.length;
// Fisher-Yates.
while (index > 1) {
final int pos = rand.nextInt(index--);
final char tmp = digits[pos];
digits[pos] = digits[index];
digits[index] = tmp;
}
return new String(digits);
}EDIT
I just realized another tweak could squeeze a little more performance out too.... consider this implementation which does not even need to create an array on each invocation, and who cares what order the digits are when you are just going to reshuffle them anyway. Thus, this saves creating a
char[] array each time as well. The only new instance is the returned String:private static final class Generator {
private final Random rand = new Random();
private final char[] digits = "0123456789".toCharArray();
public final String generate() {
int index = digits.length;
// Fisher-Yates.
while (index > 1) {
final int pos = rand.nextInt(index--);
final char tmp = digits[pos];
digits[pos] = digits[index];
digits[index] = tmp;
}
return new String(digits);
}
}
private static final ThreadLocal GENERATOR = new ThreadLocal() {
public Generator initialValue() {
return new Generator();
}
};
public static final String randomize() {
return GENERATOR.get().generate();
}Code Snippets
StringBuilder sb = new StringBuilder(number);
sb.append(String.valueOf(digits.get(randomDigit));
number = sb.toString();StringBuilder sb = new StringBuilder(10);
for (...) {
...
sb.append(digits.get(randomDigit));
...
}
return sb.toString();private static final ThreadLocal<Random> RANDOM = new ThreadLocal<Random>() {
public Random initialValue() {
return new Random();
}
}
public static final String randomize() {
final char[] digits = "0123456789".toCharArray();
final Random rand = RANDOM.get();
int index = digits.length;
// Fisher-Yates.
while (index > 1) {
final int pos = rand.nextInt(index--);
final char tmp = digits[pos];
digits[pos] = digits[index];
digits[index] = tmp;
}
return new String(digits);
}private static final class Generator {
private final Random rand = new Random();
private final char[] digits = "0123456789".toCharArray();
public final String generate() {
int index = digits.length;
// Fisher-Yates.
while (index > 1) {
final int pos = rand.nextInt(index--);
final char tmp = digits[pos];
digits[pos] = digits[index];
digits[index] = tmp;
}
return new String(digits);
}
}
private static final ThreadLocal<Generator> GENERATOR = new ThreadLocal<Generator>() {
public Generator initialValue() {
return new Generator();
}
};
public static final String randomize() {
return GENERATOR.get().generate();
}Context
StackExchange Code Review Q#35950, answer score: 18
Revisions (0)
No revisions yet.