Junit testing void against class type Java

Question

I have a problem testing a void function.

If you look at the code below, then when I wish to test this I end up having a (Deck, void) and this can't be tested (see the DeckTest)

Edit: How can I test if a deck of cards have been created i.e. public void createDeck()

public class Deck {

public Deck() {
    this.cards = new ArrayList<Card>();
}

...

public void createDeck() {
    for (Suit suit : Suit.values()) {
        for (Rank rank : Rank.values()) {
            this.cards.add(new Card(rank, suit));
        }
    }
}

...

}

Now for the junit test

public class DeckTest extends TestCase {

Deck deck = new Deck();

@Test
public void testCreateDeck() throws Exception {
    assertEquals(deck, deck.createDeck()); <--- I have (Deck, void)
}

...

}

How can I test if a deck has been created by using my "public void createDeck" in Junit?

Se link for full code

https://gist.github.com/kazyka/7a56617337d21706ec5e

The code, the problem you're facing, and the attempt at solving it, must be in the question. — JB Nizet, Jan 03 '16 at 00:26
Try to express in words what exactly (what condition, behavior, contract) you are trying to assert with your test. — Dima, Jan 03 '16 at 00:42
Since your method is impacting the `cards` variable without returning anything, you should make assertions with `cards`, e.g. asserting which particular cards you expect it to contain. — user2390182, Jan 03 '16 at 00:51
So you want me to change `public void createDeck()` to `public Card createDeck()`? But I do not need to return anything here, I just wish for a deck of cards to be created — KaZyKa, Jan 03 '16 at 00:53
Possible duplicate of [How to test void method with Junit testing tools?](http://stackoverflow.com/questions/1244541/how-to-test-void-method-with-junit-testing-tools) — kryger, Jan 03 '16 at 12:33

score 2 · Accepted Answer · answered Jan 03 '16 at 01:43

Think about what aspects you want to test.

Do you want to test...

...if 52 cards have made their way into your list?
...if you have four different suits in your list?
...if you have thirteen numerical and face cards in your list?

Ideally, you should be testing all three aspects, but the real problem comes with the way that your method is written. You can't test one of these things in isolation.

Note that the fact that this function is void actually matters little. A void method simply implies that the state of your object has changed, and to test it, you need to test the state. But, I'll get to that in a moment.

First, we need to do some reorganization of your logic.

Let's first write a method that allows us to generate all thirteen cards for a given suit. That should be straightforward enough.

Pay close attention to the visibility of this method. This is a method that we want to expose for testing, but not a method that is inherently open for the world to use. Hence, we use package-private visibility. If you have Guava, consider using @VisibleForTesting as a friendly reminder to why it's not public or private.

List<Card> generateCardsForSuit(final Suit suit) {
    final List<Card> result = new ArrayList<>();
    for(Rank rank : Rank.values()) {
        result.add(new Card(rank, suit);
    }
    return result;
}

This can be tested in isolation. Given a valid Suit, it should be able to:

generate 13 cards
contain all thirteen ranks
have a contiguous suit

Interestingly enough, we've tested 2 out of the 3 things that we care about in this particular function. Now, let's put it back into the void method.

public void createDeck() {
    for (Suit suit : Suit.values()) {
        this.cards.addAll(generateCardsForSuit(suit));
    }
}

We will have to expose a getter to retrieve the cards list, so we can actually verify things against an instance of it. Again, note the visibility of the method.

List<Card> getCards() {
    return cards;
}

Since we've comfortably tested the number of cards in a given suit, tested their uniformness, and tested that their ranks are correct, all we'd need to do is test that we generate the right number as a result of this method, using getCards() as a way to peer into the state of the object.

This also opens us up to include jokers, which would introduce 54 cards into the deck as opposed to just 52, which would be a function independent of creating all ranks and suits for the deck.

homerman · Answer 2 · 2016-01-03T01:32:44.220

1

in order to facilitate testing, you might consider re-designing along the lines of having:

a Deck class, and
a DeckFactory

if you were to do this you would be able to test that the DeckFactory does indeed to construct valid Deck instances. with that class being tested, you can just use it wherever you need a new Deck.

here's some rough boilerplate to give you the gist of what i'm saying:

public class Cards {

    public static void main(String[] args) {
        final Deck deck = new DeckFactory().newInstance();

        //... do stuff with the deck...
    }
}

class Deck {}

class DeckFactory {
    public Deck newInstance() {
        return new Deck();
    }
}

class DeckFactoryTest {
    private DeckFactory deckFactory;

    @Before
    public void setUp() {
        deckFactory = new DeckFactory();
    }

    @Test
    public void has52Cards() {
        //...
    }

    @Test
    public void has13Spades() {
        //...
    }
}

edited Jan 03 '16 at 01:32

answered Jan 03 '16 at 01:16

homerman

3,369
1
16
32

A new class for this is pretty overkill... – Makoto Jan 03 '16 at 02:21
@Makoto - that may be your opinion... just as advocating against separating concerns being an ill-advised approach would be mine. in the interest of facilitating a productive discussion, do you have any actual suggestions to offer the original poster? – homerman Jan 03 '16 at 02:42
I've already posted my answer. Personally I don't disagree around separations of concerns, but another class to do it doesn't seem like it's the *cleanest* approach. – Makoto Jan 03 '16 at 02:43
@Makoto - generally speaking, i don't mix models and "interactor" type behavior. your solution only differs from mine in that instead of having a factory class that creates Deck instances, you've bundled all that logic into the Deck class itself (with an extra method that computes a complete suit). how do you figure this is cleaner? – homerman Jan 03 '16 at 03:03

score -1 · Answer 3 · answered Jan 03 '16 at 00:52

-1

The assertEquals (deck, deck.createDeck()) line is comparing a Deck object to a void (as returned by createDeck() method) so will fail the assertion.

answered Jan 03 '16 at 00:52

Jim Potter

1
1

How else can I check if a "deck of Cards" have been created without changing my `public void createDeck()` to a `public Card createDeck()` – KaZyKa Jan 03 '16 at 00:54

Junit testing void against class type Java

3 Answers3