Count occurences of an object in a List

Question

I tried to count occurences of an object in a list, but looks like it's not working.

here is my class Card :

public class Card {
    private int value;
    private String type;
    private String color; 
}

and here, i'm trying to set up a deck of 104 cards, containing 2 occurences of each card, but looks like my condition isn't correct :

public static List<Card> InitializeDeck(){
        for(int i=0;i<104;i++){
            boolean isOk = true;
            while (isOk){
                int col = (int) (Math.floor(Math.random() * 2) + 1);
                if (col == 1) {
                    color = "Black";
                } else {
                    color = "Red";
                }

                value = (int) (Math.floor(Math.random() * 14));
                while (value == 0) {
                    value = (int) (Math.floor(Math.random() * 14));
                }

                int ty = (int) (Math.floor(Math.random() * 4));
                switch (ty) {
                    case 0:
                        type = "Spade";
                        break;
                    case 1:
                        type = "Heart";
                        break;
                    case 2:
                        type = "Diamond";
                        break;
                    case 3:
                        type = "Club";
                        break;
                }
                Card card = new Card(value, type, color);
                if(deck.isEmpty() || deck.stream().filter(line -> card.equals(line)).count()<=1){
                    deck.add(card);
                    isOk=false;
                }
            }

        }

        return deck;
    }

I get a deck of 104 cards, but with sometimes 4 occurences of the same card, or even not a single occurence, any tips ?

You need to add the code where you build the deck to see if building the deck you create a deck of 52 plus 52 different cards. Additionally you need to add the code of the `equals` method of the class `Card` — Davide Lorenzo MARINO, May 06 '19 at 14:47
How are you building that deck? If you'd just iterate over the 52 possible cards, add each one twice and at the end shuffle the deck you should be fine and not have to check for the occurence of unwanted duplicates (and you'd make sure that each card is contained exactly twice). Your approach with checking the occurences implies that you're either selecting cards randomly or let the user build the deck. — Thomas, May 06 '19 at 14:49
Btw, instead of strings for the type and color I'd suggest using enums. That will make things more robust as you'll not have to worry about using `equals()` for comparisons, unwanted types or colors etc. — Thomas, May 06 '19 at 14:52
I edited and added the code. I builded it based on random numbers, that's why i wanted to check if a card wasn't generated more than twice — OthK, May 06 '19 at 14:53
Another side note: using random numbers can make your code run a while. Let's assume your code works correctly and there's just one card missing. Getting the right combination of 3 random numbers could require a lot of tries - if my math skills aren't totally wrong getting the one right combination for 1 of 2 colors, 1 of 14 values and 1 of 4 types would be 0.89% (and that's just for _one_ card). — Thomas, May 06 '19 at 14:55
@Thomas oh you are right, at first i thought it was a good approach, not anymore lol — OthK, May 06 '19 at 14:58
@BenjaminUrquhart ok thanks, didn't knew it. I will do it now — OthK, May 06 '19 at 14:58
This is not directly related to the question, but it seems you are generating the cart type and color independently, which could lead to black diamonds or red clubs... is this intended in your case ? If not, you should consider getting the color from the type for better consistency — Gruntzy, May 06 '19 at 14:58
@Gruntzy Yes, for each type, i want to generate two colours. — OthK, May 06 '19 at 15:00
@Gruntzy good spot. Using enums that would make the enum `CardType` (to be more descriptive than just `Type`) have a member of enum type `Color`. — Thomas, May 06 '19 at 15:00
@OthK well, 2 colors, 4 types and 13 values would already give 104 different combinations. Are you sure you want that and still add cards twice? — Thomas, May 06 '19 at 15:02
@Gruntzy i saw that the enum was a better approach, but since i had no idea about how i can integrate it with my code, i gave up. But yeah, with all your suggestions, it seems like i have to look for a different approach — OthK, May 06 '19 at 15:05
How can you have red spades? Or black hearts? There is a problem in your logic there... — Yassin Hajaj, May 06 '19 at 15:05
@Thomas Oh yeah i'm so bad, it should be 208 then. But even, i still get 3 occurences for the same card. But yes, that's what i want — OthK, May 06 '19 at 15:06
@YassinHajaj I know it's weird, but that's what i wanted at first — OthK, May 06 '19 at 15:09

score 5 · Accepted Answer · answered May 06 '19 at 15:21

I'll summarize the comments on your question a little and give a brief example of how you'd build the deck without random numbers and with using enums.

First we define the enums:

enum Color {
  RED,
  BLACK;
}

enum CardType {
  SPADE, //as per Yassin's comments you'll probably want to define the type's color later on
  HEART, //you'd then use HEART(Color.RED) etc. - and provide the correct constructor
  DIAMOND,
  CLUB;
}

And the Card class:

class Card {
  private final int value;
  private final CardType type;
  private final Color color;

  //I'll omit the constructor, getters, equals and hashcode for simplicity, they should be straight forward
}

And building the deck:

List<Card> deck = new ArrayList<>(208); //we're telling the list that we expect 208 elements

//build two cards for each combination and add them to the deck
for( int value = 1; value  <= 14; value++ ) {
  for( CardType type : CardType.values() ) {
    for( Color color : Color.values() ) {
      deck.add( new Card( value, type, color ) );
      deck.add( new Card( value, type, color ) );
    }
  }
}

//shuffle the deck
Collections.shuffle( deck );

When you use immutable `Card` instances, there is no need for doing `new Card( value, type, color )` twice, you can just add the same instance to the list two times. You can go even further, when you consistently create only a single instance of each kind of `Card`, it works much like an `enum`, so you don’t need to override `equals` and `hashCode`. — Holger, May 06 '19 at 15:26
@Holger you're right we could do that. I'd still leave it that way since it might become a little too complex for the OP (who's apparently having problems with `equals` and `hashCode`) otherwise. And at some point he might want to add additional data to individual card instances (e.g. some unique id to check that they aren't "duplicated"/held by 2 players at once :) ), so having 2 instances more closely models the real world. — Thomas, May 06 '19 at 15:28
@Thomas thanks a lot !! it fixed my problem, now i will look at how i can add properly equals and hashcode — OthK, May 06 '19 at 15:37

Count occurences of an object in a List

1 Answers1