HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Random permutation of int[] and ArrayList<Integer>

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
randomandpermutationarraylistintinteger

Problem

I wrote a simple program using 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:

//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 unused

In 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.