Why is this failing the unit test in Java?

Question

I am currently learning Java. I carried out unit testing for throwing exceptions. I ran the the unit test but failed. Any take on this?

This is my code

public Card(int rank, int suit) throws SuitOutOfRangeException, RankOutOfRangeException {
    // TODO: Re-write this Constructor to throw exceptions
    try {
        if (suit > 4 || suit < 0) {
            throw new SuitOutOfRangeException();
        }
        this.suit = suit % 4;
    } catch (SuitOutOfRangeException ex) {
        System.out.println("Your input value for suit is out of the specified range");
    }

    try {
        if (rank > 12 || rank < 0) {
            throw new RankOutOfRangeException();
        }
        this.rank = rank % 13;
    } catch (RankOutOfRangeException ex) {

        System.out.println("Your input value for rank is out of the specified range");

    }
}

Part of the unit test is as the following:

@Test
public void testConstructorShouldThrowRankOutOfRangeException() {
    boolean expected = true;
    boolean actual = false;
    try {
        Card c = new Card(100,1);
        actual = false;
    } catch (SuitOutOfRangeException ex) {
        actual = false;
    } catch (RankOutOfRangeException ex) {
        actual = true;
    }
    assertEquals(expected,actual);
}

The solution is this

public Card(int rank, int suit ) throws SuitOutOfRangeException, RankOutOfRangeException {        
    if (rank <0 || rank > 12) throw new RankOutOfRangeException();
    if (suit <0 || suit >3) throw new SuitOutOfRangeException();
    this.suit = suit % 4;
    this.rank = rank % 13;
}

Your unit test would say clearly what failed. Please add the error message that showed that your test failed. — randominstanceOfLivingThing, Aug 15 '16 at 16:29
Also note that that's a horrible way of testing that no exceptions are thrown... just call the constructor, and if an exception is thrown, the test will fail anyway. If those are checked exceptions, I'd make them unchecked (do you need specific exceptions there?) and if you can't do that, just make the test method declare that it can throw them. — Jon Skeet, Aug 15 '16 at 16:31
Additionally, if `rank` is in the range `[0, 12]` and `suit` is in the range `[0, 3]`, why are you using `suit % 4` and `rank % 13` instead of just `suit` and `rank`? — Jon Skeet, Aug 15 '16 at 16:32
Possible duplicate of [What is a debugger and how can it help me diagnose problems?](http://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) — Raedwald, Aug 15 '16 at 17:23

score 1 · Answer 1 · answered Aug 15 '16 at 18:05

Lets give some more general feedback on your code; should also answer your somehow unclear "how do I do this" question.

First of all, there is absolutely no point in throwing an exception, and catching it within your constructor. Reduce that to:

public Card(int rank, int suit) { // please note: no checked exceptions!
  checkRank(rank);
  checkSuit(suit);
  this.suit = ...

With check methods that just check and throw, like

private void checkSuit(int suit) {
  if (suit < 0) throw new SuitOutOfRangeException("suit must not be negative: " + suit);
  ...

The point is: you want to put your code into really small, tiny methods. This methods have exactly one responsibility (like: checking the incoming suit for valid ranges). And: when you throw exceptions, then you include the information you need later on to understand your failures.

To test something like this, you go:

@Test(expected=SuitOutOfRangeException.class)
public void testNegativeSuit() {
   new Card(1, -1);
}

That's it. No hokus pokus with printlns, and booleans, nothing of that. All of that is waste that doesn't add anything meaningful; neither to your production logic; nor to your test cases. Please note: there is also no need for your strange assertions. You expect an exception to be thrown; and nothing else. So that is what you check for!

Talking about asserts; when you do need asserts, learn about assertThat, like

Card underTest = new Card(1, 2);
assertThat(underTest.getSuit(), is(2));

Finally: consider changing the type of suit and rank from int. Make them real classes. For sure, you might build a Rank class from an int input; but maybe, there are other options, too. Thing is: programming is about creating abstractions. If you dont use abstractions ... then you have to deal with those low-level details all the time. Like your Card class has to know how valid int-ranks should look like. If you had a Rank and Suit class, then Card would only receive Rank and Suit; and wouldnt have to worry about int ranges!

Michel Feldheim · Answer 2 · 2016-08-16T05:22:54.393

0

If you catch an exception, it can not be caught once more unless it is thrown in the catch block again.

With Junit you can do something like this

e.g.

@Rule public ExpectedException thrown = ExpectedException.none();

@Test
public void throwsException() { 
    thrown.expect(NullPointerException.class);
    thrown.expectMessage("happened");
    throw new NullPointerException("What happened?");
}

http://junit.org/junit4/javadoc/4.12/org/junit/rules/ExpectedException.html

edited Aug 16 '16 at 05:22

answered Aug 15 '16 at 16:34

Michel Feldheim

17,625
5
60
77

But you are only giving have of the example there. Without explaining what type *thrown* has ... I guess this will rather confuse newbies than help them. – GhostCat Aug 15 '16 at 18:06

Why is this failing the unit test in Java?

2 Answers2