0

I'm making a poker game, and I was trying to test and see if any player had a royal flush in their hand. Before I could test this, however, I came into a roadblock: I don't have a fast and simple way of testing each players' hands for the Ace, King, Queen, Jack, and ten in a suit.
This is my code so far:

private void findWinner() {
    // p1 hand
    List<String> p1hand = cards.subList(10, 15);
    p1hand.addAll(cards.subList(0, 2));
    System.out.println("Player 1's total hand is " + p1hand);
    // p1 hand
    List<String> p2hand = cards.subList(10, 15);
    p2hand.addAll(cards.subList(2, 4));
    System.out.println("Player 2's total hand is " + p2hand);
    // testing for royal flush
    String[] spadeRoyalFlush = {"Ace of spades", "King of spades", "Queen of spades", "Jack of spades", "10 of spades"};
    String[] heartRoyalFlush = {"Ace of hearts", "King of hearts", "Queen of hearts", "Jack of hearts", "10 of hearts"};
    String[] dimRoyalFlush = {"Ace of diamonds", "King of diamonds", "Queen of diamonds", "Jack of diamonds", "10 of diamonds"};
    String[] clubRoyalFlush = {"Ace of clubs", "King of clubs", "Queen of clubs", "Jack of clubs", "10 of clubs"};
    // p1
    boolean p1royalFlush = false;
    if(p1hand.contains(spadeRoyalFlush)) { // line 104
        p1royalFlush = true;
    }
    if(p1hand.contains(heartRoyalFlush)) {
        p1royalFlush = true;
    }
    if(p1hand.contains(dimRoyalFlush)) {
        p1royalFlush = true;
    }
    if(p1hand.contains(clubRoyalFlush)) {
        p1royalFlush = true;
    }
    if(p1royalFlush) {
        System.out.println("Player 1 got a royal flush with " + p1hand);
    }
    // p2
    boolean p2royalFlush = false;
    for(int i=0; i<p2hand.size(); i++) {
        if(p2hand.contains(spadeRoyalFlush)) {
            p2royalFlush = true;
        }
        if(p2hand.contains(heartRoyalFlush)) {
            p2royalFlush = true;
        }
        if(p2hand.contains(dimRoyalFlush)) {
            p2royalFlush = true;
        }
        if(p2hand.contains(clubRoyalFlush)) {
            p2royalFlush = true;
        }
    }
    if(p2royalFlush) {
        System.out.println("Player 2 got a royal flush with " + p2hand);
    }
} // line 165 is where I call this method

cards is an ArrayList of Strings that contains every name of each card (e.g. "Ace of spades", "3 of hearts", etc.)
When I run this code, I get the error:

Exception in thread "main" java.util.ConcurrentModificationException
at java.base/java.util.ArrayList$SubList.checkForComodification(Unknown Source)
at java.base/java.util.ArrayList$SubList.listIterator(Unknown Source)
at java.base/java.util.AbstractList.listIterator(Unknown Source)
at java.base/java.util.ArrayList$SubList.iterator(Unknown Source)
at java.base/java.util.AbstractCollection.contains(Unknown Source)
at poker.PlayGame.findWinner(PlayGame.java:104)
at poker.PlayGame.doEverything(PlayGame.java:165)
at poker.MainPoker.main(MainPoker.java:7)

I researched what that meant and I was astounded because as far as I could tell, I wasn't iterating over anything in my code.
This is the DoEverything method:

public void doEverything() {
    dealHands();
    throwCards();
    findWinner(); // line 165
}

This is the main method:

public static void main(String[] args) {
    PlayGame game = new PlayGame();
    game.doEverything(); // line 7
}
  • iterate over the array and call list.contains(currentIterationElement); right now you set it true if one of them is in the list. or, you could use the && operator in your if statements (logical AND operator) – Stultuske Jan 16 '18 at 12:41
  • You may want to rephrase the title since it hardly reflects your problem. Please show the rest of your code and mark/highlight the lines specified in the error message (line 7, 104 and 165). – Turing85 Jan 16 '18 at 12:45
  • @Turing85 I marked 104, 165 is a public void that I call this method in, and 7 is where I call the method at 165 and is my main method. –  Jan 16 '18 at 12:46
  • @TheMagician Again: please post the code. Without seeing the relevant parts, (i.e. at least methods `main(...)` and `doEverything(...)`, we cannot tell why this exception occurs. – Turing85 Jan 16 '18 at 12:48
  • @Turing85 is that better? –  Jan 16 '18 at 12:55
  • Possible duplicate of [ConcurrentModificationException thrown by sublist](https://stackoverflow.com/questions/8817608/concurrentmodificationexception-thrown-by-sublist) – Turing85 Jan 16 '18 at 12:58
  • 1
    @TheMagician Please look at the linked duplicate. TL;DR: You get the `Exception` because you strucurally modify a `subList`. The [documentation of `List::subList(int, int)`](https://docs.oracle.com/javase/9/docs/api/java/util/List.html#subList-int-int-) states: "The returned list is backed by this list, so non-structural changes in the returned list are reflected in this list, and vice-versa." – Turing85 Jan 16 '18 at 13:01
  • One tip to make it more OO, is to have a Card object, that contains 2 elements, type and suit, You *could* even make all 52 static final, and keep track of every card. Just an idea. – mmaceachran Jan 16 '18 at 15:30

2 Answers2

5

I would use List.containsAll() as documented here.

Returns true if this list contains all of the elements of the specified collection.

String[] array = ...;
List<String> list = ....;
list.containsAll(Arrays.asList(array));

Reagrding the ConcurrentModificationException at line 104, you are calling contains() on p1hand, but p1hand is from cards.subList(10, 15).

We can't see where cards comes from. You have not posted the necessary code to debug that point. But see the answer from @devpuh for more detail on calling contains() on a sub-list.

Stewart
  • 17,616
  • 8
  • 52
  • 80
  • One question: would this go in an if-statement or as the boolean's value, or would it work either way? –  Jan 16 '18 at 12:44
  • @TheMagician how should we know? This call returns `true` iff. all `String`s in `array`are also in `list` and `false` otherwise. – Turing85 Jan 16 '18 at 12:46
  • One problem: I'm still getting the error in my question. –  Jan 16 '18 at 12:57
  • @TheMagician Then you should be clear what your actual question is. If your question is about `ConcurrentModificationException`, then that should be the question; if you are asking about testing for all of `String[]` in `List`, that is what I have answered. I suggest you ask a second question, and separate the two issues. – Stewart Jan 16 '18 at 14:18
1

Be careful with List.subList(int fromIndex, int toIndex)!

Returns a view of the portion of this list between the specified fromIndex, inclusive, and toIndex, exclusive. (If fromIndex and toIndex are equal, the returned list is empty.) The returned list is backed by this list, so non-structural changes in the returned list are reflected in this list, and vice-versa. The returned list supports all of the optional list operations supported by this list.

(...)

The semantics of the list returned by this method become undefined if the backing list (i.e., this list) is structurally modified in any way other than via the returned list. (Structural modifications are those that change the size of this list, or otherwise perturb it in such a fashion that iterations in progress may yield incorrect results.)

All changes on this create List will affect the original list!

And you get a ConcurrentModificationException as soon as you try to access a sublist when the original list was structural changed. (In your case you added a new element to the list cards)

Change

List<String> p1hand = cards.subList(10, 15);
p1hand.addAll(cards.subList(0, 2));

to

List<String> p1hand = new ArrayList<>(cards.subList(10, 15)); // create a new list
p1hand.addAll(cards.subList(0, 2));

And to check if the list contains all specific cards use

p1hand.containsAll(Arrays.asList(spadeRoyalFlush));
  • 1
    "*All changes on this create List will affect the original list!*" - You neglected an important part of the documentation: *"[...] so **non-structural changes** in the returned list are reflected in this list, and vice-versa.*" – Turing85 Jan 16 '18 at 13:03
  • @Turing85 Thanks have only read a part of the JavaDoc. Now have included all the necessary Doc and improved the answer. –  Jan 16 '18 at 13:13