Java program iterating more than needed?

Question

I'm not sure if I asked the right question but that's as close as I could think of. Anyway, I'm trying to make a simplified version of Solitaire known as Elevens. Everything seemed to be fine at compilation stage, but when it ran through, it didn't work as intended. Below are the methods of my Deck and DeckTester classes that pertain to the question:

Note: I left out the headers and stuff

Deck class constructor and one of the methods:

public Deck(String[] suits, String[] ranks, int[] values){
    ArrayList<Card> cardslist = new ArrayList<Card>();
    for(String suit: suits){
        for(String rank: ranks){
            for(int value: values){
                cardslist.add(new Card(suit, rank, value));
            }
        }
    }
    size=cardslist.size();
    cards=cardslist;
}

public List<Card> getDeck(){
    return cards;
}

DeckTester main method:

public static void main(String[] args){
    String[] suitsA={"Clubs", "Diamonds", "Hearts", "Spades"};
    String[] ranksA={"Ace", "Two", "Three", "Four", "Five", "Six", "Seven", "Eight", "Nine", "Ten", "Jack", "Queen", "King"};
    int[] valuesA={1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13};
    String[] suitsB={"Lions", "Giraffes", "Wolves"};
    String[] ranksB={"One", "Two", "Three"};
    int[] valuesB={1, 2, 3};
    Deck a = new Deck(suitsA, ranksA, valuesA);
    Deck b = new Deck(suitsB, ranksB, valuesB);
    System.out.println(a.getDeck());
    System.out.println(b.getDeck());
}

This is producing a result of an array like {Ace of Clubs (point value: 1)...Ace of Clubs (point value: 13), Two of Clubs (point value: 1), etc}

I would like it to produce like a standard deck of cards.

Look at your loops again, for each suit you have 13 ranks, and for rank you have 13 values. What do you expect it to do? — azurefrog, May 02 '17 at 22:49
Get rid of the inner loop for `values`, and [iterate it in parallel](http://stackoverflow.com/q/19318707/5743988) with `ranks`. — 4castle, May 02 '17 at 22:50

score 1 · Accepted Answer · answered May 02 '17 at 22:48

You have three nested for loops, therefore you will make suits*ranks*values many cards, while you only want to make suits*ranks many cards. Therefore:

public Deck(String[] suits, String[] ranks, int[] values){
    ArrayList<Card> cardslist = new ArrayList<Card>();
    for(String suit: suits) {
        for(int i = 0; i < ranks.length; i++) {
            cardslist.add(new Card(suit, ranks[i], values[i]));
        }
    }
    size=cardslist.size();
    cards=cardslist;
}

public List<Card> getDeck(){
    return cards;
}

Keep in mind that you will have to make the ranks and values correspond in order when you pass them in to assign them correctly and make sure their lengths are the same. You will probably want to add guards for that.

Hope this helps!

i kind of figured something like that (like what the code was trying to do) i just couldn't figure out how to fix it. this helped a lot :) — user147219, May 03 '17 at 00:02

score 0 · Answer 2 · answered May 02 '17 at 22:47

To make a deck of cards (52 cards) you have 4 "suits" and 13 "ranks" for each suit. 4 * 13 = 52. But you make for each "normal card" an extra loop with each point values? You could use an enum for suit and an enum for ranks and use a switch statment (or if if you prefer that) to assign a value to the cards. You don't need an extra loop. You don't need to make an enum. Since you use strings it's perfectly possible to do it that way. But you shouldn't assign values like you're doing now. You could place the check for the value in the constructor of the card for example.

Java program iterating more than needed?

2 Answers2