HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

FuseMonsterAction implementation and unit tests

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
testsandimplementationfusemonsteractionunit

Problem

I'd like to get a general review on the following code, and I'll highlight an extra point below:

public interface PlayerAction {
    boolean isActionAllowed(final Player player);

    void performAction(final Player player) throws PlayerActionNotAllowedException;
}


public class PlayerActionNotAllowedException extends RuntimeException {
    private static final long serialVersionUID = 4656594949564649L;

    public PlayerActionNotAllowedException() {
        super();
    }

    public PlayerActionNotAllowedException(final String message) {
        super(message);
    }

    public PlayerActionNotAllowedException(final String message, final Throwable cause) {
        super(message, cause);
    }

    public PlayerActionNotAllowedException(final Throwable cause) {
        super(cause);
    }
}


public class PlayerActionNotAllowedExceptionTest {
    static {
        assertTrue(true);
    }

    @Test
    public void testConstructor() {
        new PlayerActionNotAllowedException();
    }

    @Test
    public void testConstructorMessage() {
        PlayerActionNotAllowedException exception = new PlayerActionNotAllowedException("Test");
        assertEquals("Test", exception.getMessage());
    }

    @Test
    public void testConstructorMessageCause() {
        Throwable cause = new Throwable();
        PlayerActionNotAllowedException exception = new PlayerActionNotAllowedException("Test", cause);
        assertEquals("Test", exception.getMessage());
        assertEquals(cause, exception.getCause());
    }

    @Test
    public void testConstructorCause() {
        Throwable cause = new Throwable();
        PlayerActionNotAllowedException exception = new PlayerActionNotAllowedException(cause);
        assertEquals(cause, exception.getCause());
    }
}


```
public abstract class AbstractPlayerAction implements PlayerAction {
@Override
abstract public boolean isActionAllowed(final Player player);

@Override
public void performActio

Solution

I just want to make a few minor remarks, IMO these are more "preference" than "convention", so take 'em with a grain of salt

Test "sectioning"

@Test
public void testPerformAction() {
    PlayerAction playerAction = new AbstractPlayerActionImpl();
    Player player = new Player("Test", 100, new TurnActionImpl(), new Hand(5), new Field(5), new Deck(Arrays.asList(new MonsterCard("Test", 5, 5, MonsterModus.HEALING))), new Graveyard());
    playerAction.performAction(player);
    assertEquals(1, counter.get());
}


taking this here as example:

I personally prefer really placing an empty line between the test sections: given, when, then. reformatting the example would look like that:

@Test
public void testPerformAction() {
    PlayerAction playerAction = new AbstractPlayerAction();
    Player player = new Player(/* [...] */);

    playerAction.performAction(player);

    assertEquals(1, counter.get());
}


Positive or Zero and similar:

Arguments.requirePositiveOrZero(baseMonsterCardIndex, "baseMonsterCardIndex");
Arguments.requirePositiveOrZero(fuserMonsterCardIndex, "fuserMonsterCardIndex");


let's make a definitely non-working example...

new FuseMonsterAction(1000, 1001, 1002);


Well that went somewhat wrong :(

Objects.requireNonNull(player, "player");
Hand hand = Player.getHand(); //NPE inbound!


this one looks extremely NullPointerException prone to me. IMO you trust the Player Object too much that it will not have a null hand.

Code Snippets

@Test
public void testPerformAction() {
    PlayerAction playerAction = new AbstractPlayerActionImpl();
    Player player = new Player("Test", 100, new TurnActionImpl(), new Hand(5), new Field(5), new Deck(Arrays.asList(new MonsterCard("Test", 5, 5, MonsterModus.HEALING))), new Graveyard());
    playerAction.performAction(player);
    assertEquals(1, counter.get());
}
@Test
public void testPerformAction() {
    PlayerAction playerAction = new AbstractPlayerAction();
    Player player = new Player(/* [...] */);

    playerAction.performAction(player);

    assertEquals(1, counter.get());
}
Arguments.requirePositiveOrZero(baseMonsterCardIndex, "baseMonsterCardIndex");
Arguments.requirePositiveOrZero(fuserMonsterCardIndex, "fuserMonsterCardIndex");
new FuseMonsterAction(1000, 1001, 1002);
Objects.requireNonNull(player, "player");
Hand hand = Player.getHand(); //NPE inbound!

Context

StackExchange Code Review Q#52099, answer score: 3

Revisions (0)

No revisions yet.