connect: Implement the exercise "connect" #762
Conversation
exercises/connect/connect_test.py
Outdated
| @@ -0,0 +1,113 @@ | |||
| import unittest | |||
| import connect | |||
|
|
|||
There was a problem hiding this comment.
Could you please also leave a comment stating what version of canonical-data.json the tests were adopted as discussed in #784 if aplicable?
The format is:
# Tests adapted from `problem-specifications//canonical-data.json` @ v1.0.0
|
|
||
| class ConnectGame: | ||
| def __init__(self, board): | ||
| pass |
There was a problem hiding this comment.
The solution template doesn't match its usage in connect_test.py. The solution template should only contain placeholders for all classes and methods referenced in the tests. connect_test.py does not reference connect.ConnectGame, but it does reference connect.play(), so that should be in the template.
There was a problem hiding this comment.
Thanks for pointing this out! (-:
Please check the updated one.
There was a problem hiding this comment.
@yunchih I've left a couple more comments on some minor changes.
exercises/connect/connect_test.py
Outdated
| ] | ||
|
|
||
|
|
||
| class SieveTest(unittest.TestCase): |
There was a problem hiding this comment.
This should be class ConnectTest(unittest.TestCase):
exercises/connect/connect_test.py
Outdated
|
|
||
| class SieveTest(unittest.TestCase): | ||
| def test_game(self): | ||
| for t in testcases: |
There was a problem hiding this comment.
Can you please replace t with a more meaningful name? One common method of naming foreach iterator variables is to just take the singular form of the name of the collection being iterated (i.e. for testcase in testcases).
|
@cmccandless updated as requested. |
exercises/connect/connect_test.py
Outdated
| import unittest | ||
| import connect | ||
|
|
||
| # Tests adapted from `problem-specifications//canonical-data.json` @ v1.1.0 |
There was a problem hiding this comment.
Sorry, I missed this in my last review: can you please add one additional blank line above this (2 above, 1 below) for consistency across the track?
|
Merged, thanks @yunchih! |
Implement #733