How do I stop an arraylist from printing duplicate items?-CodePudding

I'm working on a game of Go Fish for a class and whenever I run the code to deal cards I'll get duplicates. How do I remove the items from the arraylist so this doesn't happen? I've tried using the remove.() method but it hasn't worked. Any help would be greatly appreciated since I'm pretty new to Java.

import java.util.*;

public class GoFish1
{    
  public static void main(String[] args)    
    {        
      String[] arr={"Ace of Spades", "2 of Spades", "3 of Spades", "4 of Spades", "5 of Spades", "6 of Spades", "7 of Spades", "8 of Spades", "9 of Spades", "10 of Spades", "Jack of Spades", "Queen of Spades", "King of Spades", "Ace of Hearts", "2 of Hearts", "3 of Hearts", "4 of Hearts", "5 of Hearts", "6 of Hearts", "7 of Hearts", "8 of Hearts", "9 of Hearts", "10 of Hearts", "Jack of Hearts", "Queen of Hearts", "King of Hearts", "Ace of Diamonds", "2 of Diamonds", "3 of Diamonds", "4 of Diamonds", "5 of Diamonds", "6 of Diamonds", "7 of Diamonds", "8 of Diamonds", "9 of Diamonds", "10 of Diamonds", "Jack of Diamonds", "Queen of Diamonds", "King of Diamonds", "Ace of Clubs", "2 of Clubs", "3 of Clubs", "4 of Clubs", "5 of Clubs", "6 of Clubs", "7 of Clubs", "8 of Clubs", "9 of Clubs", "10 of Clubs", "Jack of Clubs", "Queen of Clubs", "King of Clubs"};
     ArrayList<String> deck= new ArrayList<String>(Arrays.asList(arr));

     
      System.out.println("Player's cards:");
     
      Random rand =new Random();        
      int randomNumber1=rand.nextInt(arr.length);
      System.out.println("\n" arr[randomNumber1]);
      deck.remove(arr[randomNumber1]);
     
      Random rand2 =new Random();
      int randomNumber2=rand.nextInt(arr.length);
      System.out.println(arr[randomNumber2]);
     deck.remove(arr[randomNumber2]);
     
       Random rand3 =new Random();
      int randomNumber3=rand.nextInt(arr.length);
      System.out.println(arr[randomNumber3]);
      deck.remove(arr[randomNumber3]);
     
       Random rand4 =new Random();
      int randomNumber4=rand.nextInt(arr.length);
      System.out.println(arr[randomNumber4]);
      deck.remove(arr[randomNumber4]);
     
       Random rand5 =new Random();
      int randomNumber5=rand.nextInt(arr.length);
      System.out.println(arr[randomNumber5]);
      deck.remove(arr[randomNumber5]);
     
       Random rand6 =new Random();
      int randomNumber6=rand.nextInt(arr.length);
      System.out.println(arr[randomNumber6]);
      deck.remove(arr[randomNumber6]);
     
       Random rand7 =new Random();
      int randomNumber7=rand.nextInt(arr.length);
      System.out.println(arr[randomNumber7]);
      deck.remove(arr[randomNumber7]);
      
     

    }
}

CodePudding user response：

First of all, you can just use the shuffle method of Collections class to shuffle the deck and print the first 7 elements:

Collections.shuffle(deck);
for (int i = 0; i < 7; i  ) {
   System.out.println(deck.get(i));
}

If you want to remove the cards from the deck yourself, then remember, that ThreadLocalRandom class is a preferred way to generate random numbers over Random.

for (int i = 0; i < 7; i  ) {
   int randomNumber = ThreadLocalRandom.current().nextInt(deck.size());
   System.out.println(deck.get(randomNumber));
   deck.remove(randomNumber);
}

CodePudding user response：

The problem is this:

You create a random number, between the interval [0, arr.length) zero is inclusive, arr.length is exclusive.

Then, you first print it from the array, then you remove it from the arraylist. They are not removed from the array. So, next time you use it to print it from the array, you may still see it.

You may use deck.size() method to create a random number, print it and remove it.

So, after you create your array list using the array, one of your code block is going to be:

Random rand =new Random();        
int randomNumber1=rand.nextInt(deck.size());
System.out.println("\n" deck.get(randomNumber1));
deck.remove(randomNumber1);

Note: Instead of removing by a reference to object with remove(Object o) method, you may use the index of that object with remove(int index) method.

Note2: When you create an arraylist using the constructor ArrayList<String>(List Arrays.asList(array)), you get different references to the same objects. If you remove them from your array, you do not remove it from the array list or vice versa.

CodePudding user response：

remove all the 7 script and use this:

for(i = 0; i < 7; i  ){
            Random rand =new Random();        
            int randomNumber=rand.nextInt(deck.length);
            System.out.println("\n" deck[randomNumber]);
            deck.remove(randomNumber);
      }

it is not recommended to use ArrayList.remove() when iterating over elements, but in this case is safe because values are random.

CodePudding user response：

Your problem is that you are drawing cards from the array - arr:

System.out.println("\n"   arr[randomNumber1]);

but you then remove the card from the deck:

deck.remove(arr[randomNumber1]);

The array always has all cards, so it's normal to have duplicates.

As a side note - why declare 7 Random instances, when you only use the first of them? The first one is enough.

Another note - since you actually need to draw from the deck, you may need to change the upper bound for the random to the size of the deck, otherwise you will eventually cause IndexOutOfBoundsException after drawing enough cards.

CodePudding user response：

If the order of the elements is not important, use a Set, namely HashSet or TreeSet. HashSet is (rather) random order, while TreeSet will order contents alphabetically.

Sets are also really fast to look up thing, i.e. using the contains() method.

If the order does play a role, keep using your ArrayList, but before insertion always check with .contains() method if item is already present.

As to your code:

I do not see where you get duplicated. You're often removing from the same indices, and
you find the max index of the list via the length of the backing array. This is not a good idea, because the list will shrink, while the array keeps its size, so sooner or later you will not effectively remove anything from the list. Instead, get the index by: int randomNumber1=rand.nextInt(deck.size()); and then simply remove from list by index: deck.remove(arr[randomNumber1]);
you are using the same code over and over again. you should replace the whole shebang with a loop:

Code:

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Random;

public class GoFish1 {
    public static void main(final String[] args) {
        final String[] arr = { "Ace of Spades", "2 of Spades", "3 of Spades", "4 of Spades", "5 of Spades", "6 of Spades", "7 of Spades", "8 of Spades", "9 of Spades", "10 of Spades", "Jack of Spades", "Queen of Spades", "King of Spades", "Ace of Hearts", "2 of Hearts", "3 of Hearts", "4 of Hearts", "5 of Hearts", "6 of Hearts", "7 of Hearts", "8 of Hearts", "9 of Hearts", "10 of Hearts", "Jack of Hearts", "Queen of Hearts", "King of Hearts", "Ace of Diamonds", "2 of Diamonds", "3 of Diamonds", "4 of Diamonds", "5 of Diamonds", "6 of Diamonds", "7 of Diamonds", "8 of Diamonds", "9 of Diamonds", "10 of Diamonds", "Jack of Diamonds", "Queen of Diamonds", "King of Diamonds", "Ace of Clubs", "2 of Clubs", "3 of Clubs", "4 of Clubs", "5 of Clubs", "6 of Clubs", "7 of Clubs", "8 of Clubs", "9 of Clubs", "10 of Clubs", "Jack of Clubs", "Queen of Clubs", "King of Clubs" };
        final ArrayList<String> deck = new ArrayList<>(Arrays.asList(arr));


        System.out.println("Player's cards:");
        final Random rand = new Random();
        for (int i = 0; i < 8 && deck.size() > 0; i  ) {
            final int randomIndex = rand.nextInt(deck.size());
            final String card = deck.get(randomIndex);
            System.out.println("\tPlayer received card: "   card);
            // you can use either one of the following
            deck.remove(randomIndex); // this is faster and will not create problems
            //          deck.remove(card); // or this. this is slower, and might conflict with duplicate items in other scenarios
        }

        System.out.println("Cards left in the deck:");
        for (final String card : deck) {
            System.out.println("\t"   card);
        }

        System.out.println("All cards dealt.");
    }
}

CodePudding user response：

There is no need to access random elements at all in this scenario.

You simply need to use Collections.shuffle(deck) which will randomize the order of the List, and then you can simply always remove the first element and use the returned String value from the remove to get name of the card.

public static void main(String[] args) {
     ArrayList<String> deck= new ArrayList<>(Arrays.asList("Ace of Spades", "2 of Spades", "3 of Spades", "4 of Spades", "5 of Spades", "6 of Spades", "7 of Spades", "8 of Spades", "9 of Spades", "10 of Spades", "Jack of Spades", "Queen of Spades", "King of Spades", "Ace of Hearts", "2 of Hearts", "3 of Hearts", "4 of Hearts", "5 of Hearts", "6 of Hearts", "7 of Hearts", "8 of Hearts", "9 of Hearts", "10 of Hearts", "Jack of Hearts", "Queen of Hearts", "King of Hearts", "Ace of Diamonds", "2 of Diamonds", "3 of Diamonds", "4 of Diamonds", "5 of Diamonds", "6 of Diamonds", "7 of Diamonds", "8 of Diamonds", "9 of Diamonds", "10 of Diamonds", "Jack of Diamonds", "Queen of Diamonds", "King of Diamonds", "Ace of Clubs", "2 of Clubs", "3 of Clubs", "4 of Clubs", "5 of Clubs", "6 of Clubs", "7 of Clubs", "8 of Clubs", "9 of Clubs", "10 of Clubs", "Jack of Clubs", "Queen of Clubs", "King of Clubs"));
     Collections.shuffle(deck);
     for (int i = 0; i < deck.size(); i  ) {
           String currentCard = deck.remove(0);
           System.out.println(currentCard);
     }
}

Here you can see deck.remove(0) will always remove the first card that was randomized until the deck is empty (I simply looped to deck.size(), but you will need to adjust the code for different hands).

This code will also avoid any issues with removing random elements as you will never attempt to remove a value at an index that is higher than the current total of elements anymore, because you are always removing the first element.

Note that you also do not need to create an array yourself at all, you can initialize a List using Arrays.asList to create a temporary array in Java 8, or use List.of in Java 9 .

CodePudding user response：

You are randomizing over the String[] to identify what to remove from the List. So you keep reporting that you are removing a Card when it is already removed. Instead, you may randomize over the List itself.

Note that anytime you find yourself repeating the same code you should strongly consider moving that logic into a method. In addition, consider creating classes for Deck, Hand, and so forth and defining behaviors (i.e. methods) to execute behavior.

public String drawFromDeck() {
  if (deck.isEmpty()) { // assumes deck is instance variable
    throw new Exception("Deck is empty!");
  }
  int index=rand.nextInt(deck.size()); // assumes rand is an instance variable
  String card = deck.remove(index);
  return card;
}

However, I think you may find that a better approach is to populate the List, shuffle it, and then just draw from the front..

final String[] cards = ....
List<String> deck = ... fill deck from cards array
Collections.shuffle(deck);

Since we've shuffled the deck we can always draw from the front of the deck rather than randomizing our draw.

public String drawFromDeck() {
  if (deck.isEmpty()) {
    throw new RuntimeException("Deck is empty!");
  }
  return deck.remove(0);
}