Cartesian\Point, Cartesian\Plane and Turns. #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also changes the requirement as a development requirement, since it is only needed for tests.
A cartesian plane has limits (x and y axis) and is inteded to hold the game state. A point in the cartesian plane is stored in an SplFileObjectStorage so we can manage what is holded into any given point, in my mind, that "bag" holding stuff (who own a point on the plane, what it has) for a point should be called "Tile".
When an index does not exists in the arena array, a notice was being raised.
Favoring always Player 1, this transposes the relevant contents of `playgame.php` to an integration test in order to provide refactor capabilities. Player 1 is always favored because he moves later, overwriting the move from player 0 on game turns. He rarely loses, for now, I think this test should survive the ocasional failures (they are rare) as it is our only end-to-end test.
A turn is observable as a subject, the game is an observer of it and is updated each turn occours. The `Turn` class is also an iterator, each iteration increases one turn util the limit of turns is reached. Every turn notifies the observers. The Game class is made iterable, returning the Turn object of the game so we can do update the game state every turn.
Explain variables as I dig through the code to understand the mechanics. Took a lot of opionated actions on naming as they likely convey better the methods behaviours (also raised on @todos). Tests were renamed also, to better express the expectations and canditions of each one.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The commits express everything I wanted to (or thought at the time). I took the tests @iannsp made and put them inside an Integration suite, also explained them facilitating understanding of the game (as I progressed through the code).
I did not made the refactor on
Arenawith theCartesianPlanebecause I would like some feedback before procceding.PS: some of my opinions were raised on
TODOs, try grepping for them.PPS: I used snake case on test names (instead of camel case) as I think they are more readable for long test names.