sli-fox/HearthSim — pull request #1 by sli-fox (2014-12-02 01:59:07+00:00)
Title: CardAction implementation

Change 0/14: Implementation of CardAction classes
The goal of these commits is to remove some of the dependencies between the Card model and other game entities such as the HearthTreeNode or board state.
The underlying reason for this attempt at decoupling is that the Card model should serve as a model of a Card only and have no knowledge of board states or other game entities. This adheres to the concept of MVC.
Currently, board states are passed into a Card's useOn() function and changes are then made to the board state that reflect the usage of the Card. The updated board state is then returned.
The CardAction delegate class was created to handle the usage of a Card without the Card having to know about board states. Each Card will have a CardAction that handles the updating of a board state based on the usage of the Card.
Special cards that overrided the original useOn() from Card because of their special usages were given specific CardAction classes that extend the base CardAction classes.
An unit test for CardAction was made (TestCardAction) and all exiting unit tests for Cards were updated to use CardAction's methods. All tests pass with the implementation of CardAction.
NOTE: Diffs for the following commits can be seen via Github by clicking on each commit below.

Change 1/14: Initial creation of CardAction class
CardAction was created with the essential card usage functions from the Card class. The main functions in question are the useOn() ones -- the motivation behind this refactoring is to create a delegated class CardAction that implements useOn() instead of having the Card model deal directly with the HearthTreeNodes (board states), players sides, and decks.
The CardAction class will also serve as a base for specific CardAction classes needed for special cards that override various useOn() and useOn()-related functions.

Change 2/14: Added CardAction attribute to Card class
Cards will now own a CardAction and pass itself into the CardAction constructor in order to give CardAction access to its properties.
This association also matches a Card to its CardAction.

Change 3/14: Added getter for CardAction
The addition of a getter for a Card's CardAction instance is necessary to be able to call the CardAction's useOn() in the code base.

Change 4/14: Added overloaded useOn()
Overloaded useOn() functions from Card were added to the CardAction class. The attempt here is to delegate all forms of useOn() to CardAction.

Change 5/14: Added new CardAction test
A separate JUnit test based on the test for the board model was made specifically to test the new CardAction functionalities. The reasoning behind this test is to ensure that the delegation of useOn() to CardAction did not adversely affect the code. This is done prior to changing the entire code base's implementation of useOn().

Change 6/14: New classes for Observer pattern
An attempt to add an Observer to the implementation of CardAction was made. This commit includes the Subject and Observer classes.
As explained later on in this patch set, a design decision was made to not use the Observer pattern as it not benefit the implementation of CardAction. These classes will later be removed.

Change 7/14: Replaced Card's useOn() and associated methods with the new delegated methods from CardAction
Now that the CardAction class has been tested, the old useOn() and related functions from card are replaced with calls to the CardAction's useOn() functions. This completes the delegation of responsibility pf useOn() from Card to CardAction.

Change 8/14: Finalized CardAction's methods
To pass the existing unit tests, the use_core() function was left in Card and CardAction utilizes Card's use_core() in its implementation of useOn(). This is not the most desirable refactoring solution, since use_core() was left implemented in Card. The move of the use_core() function to CardAction did not break the TestCardAction unit test but did break all of the existing unit tests for each card that were written by the original developers. A much more systemic refactoring would have been needed to be able to pass all of the original unit tests, thus, use_core() was left in Card and all other functionalities related to useOn() was delegated to CardAction.

Change 9/14: Added child classes of CardAction that allowed for special card usages
Three different cards (Nightblade, ArchmageAntonidas, and VioletTeacher) required specialized CardActions because when these cards are used, additional special effects are enacted on the board state.
Thus, three child classes of CardAction were made, one for each of these specialized cards and each of these child CardAction classes overrided the necessary useOn() or useOn() related function in their respective implementations. These overrided function implemented the special functionalities of the cards.

Change 10/14: Removed useOn() and related functions from Card class that were delegated to CardAction
All methods delegated to CardAction were removed from Card in order to preserve the Card model.

Change 11/14: Minions with special card usages were associated with specific CardAction child classes
Each of the three cards mentioned in change 9/14 now has a specific CardAction instead of the base CardAction. For example, the Nightblade card's action was instantiated as a CardActionNightBlade object.

Change 12/14: Additional replacement of Card's useOn() and associated methods with the new delegated methods from CardAction
After the removal of useOn() and related functions from Card, more places with Card methods had to be replaced with CardAction functions.

Change 13/14: Changed all tests using Card's useOn() to the new CardAction methods
All of the original developers' unit tests were updated to use the new CardAction methods. The tests were run and all of them passed with the new CardAction implementation.

Change 14/14: Removal of the Observer pattern
In the course of this refactoring, a design decision was made to not have an Observer pattern. There was no need to implement an Observer pattern to decouple Card from the board state and other entities. Since the useOn() function family was delegated to a CardAction object, the Card model was separated from the other game entities -- CardAction remained the association between Card and the board state. Furthermore, CardAction's useOn() method was called where it was needed to return an updated board state (after the usage of a card). Thus, the client class would get the card to be used, call useOn() of that card's CardAction instance and pass in the current board state. CardAction's useOn() would manipulate the data within the board state and return an updated board state.