patternjavaModerate
Random permutation of int[] and ArrayList<Integer>
Viewed 0 times
randomandpermutationarraylistintinteger
Problem
I wrote a simple program using
Random arrays using brute force:
3 5 2 9 7 8 10 1 4 6
4 10 5 2 3 9 6 1 7 8
7 9 1 10 3 8 5 4 2 6
5 3 8 9 10 2 6 7 1 4
8 5 9 7 10 6 4 2 1 3
4 10 2 9 8 6 3 7 5 1
8 9 10 2 5 4 6 7 3 1
7 2 4 9 5 10 6 8 1 3
9 2 8 1 10 7 6 4 5 3
8 1 10 3 9 7 4 2 5 6
This is my BrutePermutationGenerator file:
This is my SmartPermutationGenerator file:
```
import java.util.ArrayList;
import java.util.Random;
public class SmartPermutationGenerator
{
private int size;
private Random rand = new Random();
public SmartPermutationGenerator()
{
this.size = 10;
}
public ArrayList getRandomPermutation()
{
ArrayList unused = new Ar
int[] and ArrayList which aims to get a random permutation output between 1 to 10, where each number will not be repeated in each line of output (each line of output will have number 1 until 10 in a different order). Both classes work nicely with the desire output. Is there any other possible way to shorten my code?Random arrays using brute force:
3 5 2 9 7 8 10 1 4 6
4 10 5 2 3 9 6 1 7 8
7 9 1 10 3 8 5 4 2 6
5 3 8 9 10 2 6 7 1 4
8 5 9 7 10 6 4 2 1 3
4 10 2 9 8 6 3 7 5 1
8 9 10 2 5 4 6 7 3 1
7 2 4 9 5 10 6 8 1 3
9 2 8 1 10 7 6 4 5 3
8 1 10 3 9 7 4 2 5 6
This is my BrutePermutationGenerator file:
import java.util.Random;
public class BrutePermutationGenerator
{
public int[] getRandomPermutation()
{
Random rand = new Random();
int[] array = new int[10];
int r = 0;
int count = 0;
boolean fill;
int low = 1;
int high = 10;
int range = high - low + 1;
do
{
fill = true;
r = rand.nextInt(10) + 1;
for (int i = 0; i < array.length; i++)//loop for random number between 1 to 10
{
if (array[i] == r)
{
fill = false;
}
}
if (fill == true)
{
array[count] = r;
count++;
}
}
while (count < 10);
for (int i = 0; i < array.length; i++)//loop for element in array
{
System.out.print(array[i] + " ");
}
return array;
}
public void nextPermutation()
{
for (int i = 0; i < 10; i++) //loop for permutation 10 times
{
getRandomPermutation();
System.out.println();
}
}
}This is my SmartPermutationGenerator file:
```
import java.util.ArrayList;
import java.util.Random;
public class SmartPermutationGenerator
{
private int size;
private Random rand = new Random();
public SmartPermutationGenerator()
{
this.size = 10;
}
public ArrayList getRandomPermutation()
{
ArrayList unused = new Ar
Solution
Use for-each
Use for-each idiom to loop over an array when possible, and you can drop this comment:
Like this, and without the pointless comment:
Use a different list implementation for
In
you're using an
and randomly remove items from it.
An
and removing items from arrays is an expensive operation:
elements after the removed element must be moved to fill the gap.
You can improve that point by using a
where removal of elements is cheap.
Use boolean expressions directly without
You don't need to write
This is exactly the same but shorter, simpler, intuitive:
Remove unused code
These variables are completely unused, remove them:
Printing arrays
In both implementations, you're using a
There's are easier ways.
The
so you can print them directly:
To print arrays in the same format, you can use
Formatting
Your code doesn't follow the common formatting used by auto-formatting of editors like Eclipse, IntelliJ, NetBeans. Instead of this:
Following the same style as recommended by these tools,
your code would look like this:
Use for-each idiom to loop over an array when possible, and you can drop this comment:
//loop for element in array
for (int i = 0; i < array.length; i++) {
System.out.print(array[i] + " ");
}Like this, and without the pointless comment:
for (int item : array) {
System.out.print(item + " ");
}Use a different list implementation for
unusedIn
SmartPermutationGenerator,you're using an
ArrayList for the unused numbers,and randomly remove items from it.
An
ArrayList is backed by an array,and removing items from arrays is an expensive operation:
elements after the removed element must be moved to fill the gap.
You can improve that point by using a
LinkedList instead,where removal of elements is cheap.
public List getRandomPermutation() {
List unused = new LinkedList<>();
for (int i = 0; i permutation = new ArrayList<>();
for (int k = 0; k < size; k++) {
int pos = random.nextInt(unused.size());
permutation.add(unused.get(pos));
unused.remove(pos);
}
return permutation;
}Use boolean expressions directly without
==You don't need to write
== true when evaluating boolean expressions like this:if (fill == true) {This is exactly the same but shorter, simpler, intuitive:
if (fill) {Remove unused code
These variables are completely unused, remove them:
int low = 1;
int high = 10;
int range = high - low + 1;Printing arrays
In both implementations, you're using a
for loop to print arrays.There's are easier ways.
The
toString method of List implementations gives a nice representation,so you can print them directly:
List list = Arrays.asList(1, 2, 3, 4, 5);
System.out.println(list);
// prints: [1, 2, 3, 4, 5]To print arrays in the same format, you can use
Arrays.toString:int[] arr = {1, 2, 3, 4, 5};
System.out.println(Arrays.toString(arr));
// prints: [1, 2, 3, 4, 5]Formatting
Your code doesn't follow the common formatting used by auto-formatting of editors like Eclipse, IntelliJ, NetBeans. Instead of this:
do
{
fill = true;
r = rand.nextInt(10) + 1;
for (int i = 0; i < array.length; i++)//loop for random number between 1 to 10
{
if (array[i] == r)
{
fill = false;
}
}Following the same style as recommended by these tools,
your code would look like this:
do {
fill = true;
r = rand.nextInt(10) + 1;
for (int item : array) {
if (item == r) {
fill = false;
}
}Code Snippets
//loop for element in array
for (int i = 0; i < array.length; i++) {
System.out.print(array[i] + " ");
}for (int item : array) {
System.out.print(item + " ");
}public List<Integer> getRandomPermutation() {
List<Integer> unused = new LinkedList<>();
for (int i = 0; i < size; i++) {
unused.add(i + 1);
}
List<Integer> permutation = new ArrayList<>();
for (int k = 0; k < size; k++) {
int pos = random.nextInt(unused.size());
permutation.add(unused.get(pos));
unused.remove(pos);
}
return permutation;
}if (fill == true) {if (fill) {Context
StackExchange Code Review Q#69929, answer score: 10
Revisions (0)
No revisions yet.