Skip to content

tests, small comments and modifications, and start of new strategy #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mayaekd
Copy link
Owner

@mayaekd mayaekd commented Jun 20, 2020

  • Created test_C4Game.py to test some things in C4Game.py. Currently passes all tests.
  • Added comments
  • Added (player) names to parameters for creating a C4Game
  • Fixed some small typos
  • Idea for new strategy is started in C4Game.allPossibleWins.
    • allPossibleWins is not yet implemented but there is a large comment describing what it should do and some pseudocode
    • The idea comes from the observation that there really aren't that many distinct lines of 4 (or however many) that you can complete to win. The idea is to keep a set of all the possible lines of 4 (or however many) that could be made, and every time during play that one of those possibilities is lost, remove it from the set. Trying to win can be based on trying to move toward completing the lines still in the set.

- Created test_C4Game.py to test some things in C4Game.py.  Currently passes all tests.
- Added comments
- Added (player) names to parameters for creating a C4Game
- Fixed some small typos
- Idea for new strategy is started in C4Game.allPossibleWins.
    - allPossibleWins is not yet implemented but there is a large comment describing what it should do and some pseudocode
    - The idea comes from the observation that there really aren't that many distinct lines of 4 (or however many) that you can complete to win.  The idea is to keep a set of all the possible lines of 4 (or however many) that could be made, and every time during play that one of those possibilities is lost, remove it from the set.  Trying to win can be based on trying to move toward completing the lines still in the set.
@mayaekd mayaekd requested a review from joenelsong June 20, 2020 07:13
self.assertEqual(game.turn, 0)
self.assertEqual(len(game.playerArray), game.players)
# Tests that playerArray comes out as expected
player0Exp = Player(0, "Maya")
Copy link
Collaborator

@joenelsong joenelsong Jun 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Exp on the end of this variable name seems confusing, also would be better to Hard code the test values instead of running them through a Player constructor, just incase the error is int he Player constructor

Copy link
Collaborator

@joenelsong joenelsong Jun 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah I see, but the Color comes from the Game, so maybe something like this makes more sense.
testP0Name = "maya"
testP1Name = "joey"
player0Exp = Player(0, testP1Name)
...

and then reuse the testPXNames at the top too.

But not a big deal :P

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain to me why naming the strings like that is better? :)
Exp stood for expected in my mind. What did it evoke for you?

Copy link
Collaborator

@joenelsong joenelsong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job with the test cases! I left a comment or two.
Also C4Game.py no longer runs but we can fix that later anyway, it should probably take command line arguments to start it anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants