Beta

Hearts (Card Game) Kata 3 of 3

7p4songer
Description
Loading description...
Puzzles
Games
View
AllIssues5Questions1Suggestions1Show Resolved
  • Please sign in or sign up to leave a comment.
  • Voile Avatar

    The reference solution is still incorrect, it determines the wrong person when someone shoots the moon.

    • p4songer Avatar

      This comment has been hidden.

    • Voile Avatar

      I can't see the spoilers. But I do know that the reference solution is taking the leader of the first hand as the person who shot the moon, instead of the actual person shooting the moon.

    • Voile Avatar

      Okay, my solution passes the kata now, so this should be fixed.

      Issue marked resolved by Voile 2 years ago
    • p4songer Avatar

      Great! Sorry that my comment didn't make it to you, I'm still learning how this side of codewars works. My mistake was a typo. Let me know if you have any other issues!

  • Voile Avatar
     
    @test.it("Scores are managed internally")
def try_cheat():
    game = Hearts()
    try: 
        game.alex_score = 500
        game.bob_score = -100
        game.chris_score = 250
        game.dave_score = 0
        test.assert_equals('I can modify your scores externally', 'score modification can only happen inside of a class instance')
    except AttributeError:
        test.assert_equals(True, True)
@test.it("Winner and loser are accurate")
def who_win():
    game = Hearts()
    try:
        game._alex_score = 25
        game._chris_score = 24
        game._dave_score = 1
        test.assert_equals(game.winner(), 'Bob is winning with 0 points!', 'Incorrect winner return value with low score')
        test.assert_equals(game.loser(), 'Alex is losing with 25 points!', 'Incorrect loser return value with low score')
        game._alex_score = 101
        test.assert_equals(game.winner(), 'Bob won with 0 points!', 'Incorrect winner return value with high score')
        test.assert_equals(game.loser(), 'Alex lost with 101 points!', 'Incorrect loser return value with low score')
    except AttributeError:
        test.assert_equals('AttributeError', 'player scores should be attributes, and winner/loser should be a method')

    This is an incorrect usage of Python classes. _ is meant to be weak "internal use" indicator; it's not meant to be used by outside API as a field directly. The tests shouldn't even know the existence of such "internal use" fields, let alone testing their behaviours.

    More importantly, if scores are intended to be modified, then creating this hurdle is pointless. Python is not Java after all. But if scores aren't intended to be modified, as what this interface would infer, then this shouldn't even be tested, as then the tests would also need to test score validation (what if I set someone's score to -100?), and whether games break if scores are set to different values during a game, etc. (Besides, these _ fields aren't use anywhere else other than this part anyway, so these tests do not contribute to anything about the behaviour of the class, aka they're unhelpful as unit tests.)

    Also, the part about the name of these getters/fields aren't mentioned anywhere, and can only be inferred by the tests.

    p.s The tests also swallow exceptions, which is bad practice. You'll at least need to do something like

    except AttributeError as e:
    print(e)
    ...
    • p4songer Avatar

      Thanks for the great explanation of what's going wrong here. I just want to make sure that I'm hearing you right, because my instinct is to delete these tests based off of your explanation.

      More importantly, if scores are intended to be modified, then creating this hurdle is pointless. Python is not Java after all. But if scores aren't intended to be modified, as what this interface would infer, then this shouldn't even be tested, as then the tests would also need to test score validation

      It sounds like if I've described the goal well enough, these tests are only effective in testing if the user has written something that uses a getter method, which sounds unneccesary based on the rest of your description. Especially since I've allowed the posibility of exceptions in the testing, (which is bad practice, as you informed me). I just want to make sure that the way I describe the class methods is easy enough to understand. I don't use classes, so a lot of the specifics of what I need to test is a little forign to me.

    • Voile Avatar

      Especially since I've allowed the posibility of exceptions in the testing, (which is bad practice, as you informed me)

      Testing for exceptions isn't the problem, swallowing the exception is the problem. When the tests hit that spot instead of the actual exception message like

      'Hearts' object has no attribute '_alex_score'

      we get

      'AttributeError' should equal 'player scores should be attributes, and winner/loser should be a method'

      As you can see, crucial debugging information is lost. In general tests shouldn't be unhelpful to users for no particular reasons.

      It sounds like if I've described the goal well enough, these tests are only effective in testing if the user has written something that uses a getter method, which sounds unneccesary based on the rest of your description

      The problem here is that these tests are testing implementation details unrelated to the desired behaviours of the class, which goes against what unit tests are about. What a Hearts object really need to tell are the scores of each player; so the public-facing API of Hearts only needs to give a getter. There are no requirements that it should allow setting the scores directly as well (as can be seen by the tests).

      The previous API of returning every player score as a dict works perfectly fine. Why is it changed to the current version?

      these tests are only effective in testing if the user has written something that uses a getter method

      The second test group isn't testing the getter, it's testing the setter. And property defines a getter only. So at least there is a hidden requirement of "oh, also you need to define a setter of the same name but with _ prepended"; but then there are further questions of why a setter needs to exist if test code isn't using it anyway, and why the setter can't just have the same name as the getter.

    • p4songer Avatar

      Thanks for your clarification. I see now that my idea of python classes was wrong. Where I thought that the prefixed underscore and getter was mandatory for correct class functionality (although, this might be needed for the testing class to prevent cheating), I see now that there are many ways to write correct code for the problem. The old tests were confusing because I was forcing code to be written in one style, where ultimately, I only wanted to test the side effects. You've taught me a lot about classes through this conversation, so thank you for your patience!

      These tests for the confusing methods have been removed.

      Testing for exceptions isn't the problem, swallowing the exception is the problem. When the tests hit that spot instead of the actual exception message like 'Hearts' object has no attribute '_alex_score'

      we get

      'AttributeError' should equal 'player scores should be attributes, and winner/loser should be a method' As you can see, crucial debugging information is lost. In general tests shouldn't be unhelpful to users for no particular reasons.

      I understand now the difference of allowing errors in testing, and swallowing errors. I have modified the section of testing that will be prone to errors, and have fixed the way it will return error messages. Once again, I will republish so you can see the changes, and if there's anything else that needs to be done, just let me know!

      Thanks again for your help!

      Issue marked resolved by p4songer 2 years ago
  • Voile Avatar

    winner/loser does not specify what happens if multiple players tie for the lowest score/highest score, or if multiple players loses in the same round.

    • p4songer Avatar

      This part of the kata has been removed.

      The previous API of returning every player score as a dict works perfectly fine. Why is it changed to the current version?

      I added this because initially I thought that including this method would be simple, and it fit the theme of the class overall. However, after looking into how to clarify your point, I realize that fully explaining this would just add more complexity that isn't related to the inital goal of the kata. Hence the removal of this requirement.

      I have also reverted the requirement to simply return a python dictionary containing the players scores, once a method is called. I agree that this is the simplest and best way to finish this kata.

      Thanks again for your keen eye!

      Issue marked resolved by p4songer 2 years ago
  • Voile Avatar

    Reference solution has incorrect logic in determining whether someone has shot the moon, and when it happens, who shot the moon. Hence causing all the discrepency between reference solution and our solutions.

    • p4songer Avatar

      Thanks for looking into this even more! I'm just about to start re-writing this after a good night's sleep, so I'll keep an eye out for this in particular.

    • p4songer Avatar

      Hello again! I am marking this particular issue as resolved. I would still love it if you reviewed the rest of the new code, but for this particular issue, I am confident in the new logic for determining when someone has shot the moon.

      Issue marked resolved by p4songer 2 years ago
  • Voile Avatar

    The current kata design is insane with the functions having to use external states but no available anywhere, hence using a global state SCORE. The tests are deeply coupled with it, and the users can do various things to break the tests by using other objects as SCORE.

    This anti-pattern has a simple cure, which is known as "using a class".

    To actually run games that need to keep track of states, it should belong to a class (something named Hearts), with methods that run games, and finally a method that returns scores. Tests are only resetting SCORE each time anyway, which equals to creating a new instance of said class.

    • p4songer Avatar

      Your suggestion of using a class does sound like the simplest solution to my problems. I've been re-writing the code for this kata anyway, so I will un-publish for now, and I'll let you know once I have something less crazy up and running! Thanks for your feedback.

    • p4songer Avatar

      I've just about got a working class fixed up, and upon re-reading this a few times, I think I'm on the right track, but I want to make sure I'm fully understanding your recommendation.

      To actually run games that need to keep track of states, it should belong to a class (something named Hearts), with methods that run games, and finally a method that returns scores. Tests are only resetting SCORE each time anyway, which equals to creating a new instance of said class.

      In other words, the task of this kata should be described as: create a class Hearts such that it keeps track of score, and has methods to evaluate score and such (per the other rules we already have). Then unit tests can be based off of each instance of that class, which will be able to self regulate better than depending on a global dictionary. With each of these instances, we have more secure tests, as well as simpler, more organized unit testing parameters.

      This anti-pattern has a simple cure, which is known as "using a class".

      If that all sounds correct, check out the new code! most of it is still a work in progress, but with your wise guidance, I think I'm finally heading the right direction with this. If i'm heading the definite wrong direction, just leave the issue unresolved, and I'll re-evaluate again.

      Thanks again for your help!

    • p4songer Avatar

      Hello again! I think I have sucessfully resolved this issue. The scope of the kata has changed quite a bit from the original, but I think that this makes for a better, more enjoyable kata anyway. I'll wait to publish again until you have had a chance to review everything new, and find the issue resolved to your standards. I'm still learing how parts of codewars work, so I'm not sure if you'll be able to do an actual test with the new testing. Either way, just let me know if there's anything else you need me to do to resolve this issue!

    • p4songer Avatar

      I'm marking this as resolved. I'll re-publish, and tackle any other issues as they come up.

      Issue marked resolved by p4songer 2 years ago
    • Voile Avatar

      Initial code uses _player_score but the sample tests uses alex_score, bob_score, chris_score and dave_score. They don't match.

    • p4songer Avatar

      That's a good point. I think for clarity, I could either remove this example from the initial code, or use alex_score as the example instead. I think I'm leaning towards just removing this example since this is something seems to be pretty common in python classes. (I'm not sure though, this is only the 2nd class I've ever written.)

      For now, I've changed it to be alex_score, but if you think it's better to remove, let me know!

  • Fbasham Avatar

    If it helps, here's a full test that fails for me:

    In the following example our answers differ on the 6th round.
    I get a score of {'Alex': 19, 'Bob': 26, 'Chris': 74, 'Dave': 37} and you get {'Alex': 45, 'Bob': 36, 'Chris': 72, 'Dave': 55}

    Each section is:

    ROUND #
all_players_hands_at_start_of_game  
played_cards  winning_card  index_of_winning_card  
  ...
played_cards  winning_card  index_of_winning_card  
SCORE
collected_point_cards_by_name

? possible_comment_from_your_tests (ex. alex shot the moon!) ?

-----------------------------------------------------------------------------------------------------------

ROUND #1
[['3C', 'AS', '8S', '5D', 'AH', 'JD', '4D', '2C', '8H', '10D', 'AC', '6C', 'JH'], ['5C', 'QH', '9S', 'AD', '5S', '2S', 'QC', '2H', '10H', '3D', '4S', '10S', '5H'], ['6D', 'KC', '9C', '7C', 'JS', '7H', '3S', 'QD', '8C', '7S', 'QS', '10C', '6H'], ['JC', '9D', 'KS', '6S', '4C', '2D', '3H', 'KD', '9H', 'KH', '8D', '7D', '4H']]
['2C', '5C', 'KC', 'JC'] KC 2
['5D', 'AD', '6D', '9D'] AD 1
['AH', 'QH', '7H', '3H'] AH 0
['3C', 'QC', '9C', '4C'] QC 1
['AS', '9S', 'JS', 'KS'] AS 0
['8S', '5S', '3S', '6S'] 8S 0
['JD', '3D', 'QD', '2D'] QD 2
['AC', '2H', '7C', '9H'] AC 0
['4D', '10H', 'QS', 'KD'] KD 3
['8H', '5H', '6H', 'KH'] KH 3
['10D', '2S', '8C', '8D'] 10D 0
['6C', '4S', '10C', '4H'] 10C 2
['JH', '10S', '7S', '7D'] 10S 1
{'Alex': 6, 'Bob': 1, 'Chris': 1, 'Dave': 18}
{'Alex': ['AH', 'QH', '7H', '3H', '2H', '9H'], 'Bob': ['JH'], 'Chris': ['4H'], 'Dave': ['10H', 'QS', '8H', '5H', '6H', 'KH']}

ROUND #2
[['6C', '10S', 'JC', '5D', '6S', '2H', '9C', 'AC', 'QC', '5S', 'QD', 'KS', '2S'], ['KC', '4D', '5C', 'AH', '3H', '2C', '6H', '8H', '8C', 'AD', 'JS', 'KH', '2D'], ['JH', '8D', '3C', 'KD', 'QS', '5H', '10D', 'QH', '3S', '10C', '10H', '4H', '4C'], ['3D', '7H', '7C', '7D', '4S', '7S', '9S', '9H', '6D', '8S', 'JD', '9D', 'AS']]
['6C', '2C', '3C', '7C'] 7C 3
['5D', '4D', '8D', '3D'] 8D 2
['2H', 'AH', 'JH', '7H'] AH 1
['JC', 'KC', '10C', '9H'] KC 1
['9C', '5C', '4C', '7D'] 9C 0
['10S', 'JS', 'QS', '4S'] QS 2
['QD', 'AD', 'KD', '6D'] AD 1
['6S', '3H', '5H', '7S'] 5H 2
['AC', '2D', '10D', 'JD'] JD 3
['5S', '6H', '3S', '9S'] 9S 3
['KS', '8H', 'QH', '8S'] KS 0
['QC', '8C', '10H', '9D'] QC 0
['2S', 'KH', '4H', 'AS'] AS 3
{'Alex': 9, 'Bob': 6, 'Chris': 16, 'Dave': 21}
{'Alex': ['8H', 'QH', '10H'], 'Bob': ['2H', 'AH', 'JH', '7H', '9H'], 'Chris': ['QS', '3H', '5H'], 'Dave': ['6H', 'KH', '4H']}

ROUND #3
[['KH', '8C', '2D', '8D', '5D', 'QH', 'KS', '4C', 'JH', 'AH', '3C', '5H', 'JC'], ['QS', 'JS', '2H', '4S', '2S', '3D', '6D', 'QC', '2C', '9C', 'KC', '6C', '7C'], ['AS', 'AC', 'JD', 'KD', '8S', '7H', '9D', '7D', 'AD', '5C', '10D', '7S', '6H'], ['3H', '4D', '10C', '10H', '9H', '10S', '4H', 'QD', '6S', '3S', '8H', '5S', '9S']]
['8C', '2C', 'AC', '10C'] AC 2
['KS', 'QS', 'AS', '10S'] AS 2
['2D', '3D', 'JD', '4D'] JD 2
['8D', '6D', 'KD', 'QD'] KD 2
['KH', 'JS', '8S', '6S'] JS 1
['QH', '2H', '7H', '3H'] QH 0
['5D', '4S', '9D', '10H'] 9D 2
['JH', '2S', '7D', '9H'] 7D 2
['AH', 'QC', 'AD', '4H'] AD 2
['4C', '9C', '5C', '8H'] 9C 1
['3C', 'KC', '6H', '3S'] KC 1
['JC', '6C', '10D', '5S'] JC 0
['5H', '7C', '7S', '9S'] 5H 0
{'Alex': 14, 'Bob': 9, 'Chris': 34, 'Dave': 21}
{'Alex': ['QH', '2H', '7H', '3H', '5H'], 'Bob': ['KH', '8H', '6H'], 'Chris': ['QS', '10H', 'JH', '9H', 'AH', '4H'], 'Dave': []}

ROUND #4
[['5S', '6H', '5H', '3H', '5C', '10C', 'AH', '5D', '7D', '9D', '6S', '4S', 'AC'], ['7H', 'QC', 'KC', '8S', '9S', '10H', 'AS', '2D', '4D', '6C', '3S', '3D', '7C'], ['6D', '8H', '3C', '7S', 'QS', '2S', 'JC', 'KH', '4C', '8D', '2H', 'JH', '10S'], ['KD', 'QH', '2C', 'QD', 'KS', '8C', 'JS', '9H', '10D', 'AD', 'JD', '9C', '4H']]
['5C', 'QC', '3C', '2C'] QC 1
['6H', '7H', '8H', 'QH'] QH 3
['5D', '2D', '6D', 'KD'] KD 3
['7D', '4D', '8D', 'QD'] QD 3
['5S', '8S', '7S', 'KS'] KS 3
['10C', 'KC', 'JC', '8C'] KC 1
['6S', '9S', 'QS', 'JS'] QS 2
['4S', 'AS', '2S', '9H'] AS 1
['5H', '10H', 'KH', '4H'] KH 2
['AC', '6C', '4C', '9C'] AC 0
['3H', '3S', '2H', '10D'] 3H 0
['AH', '3D', 'JH', 'AD'] AH 0
['9D', '7C', '10S', 'JD'] JD 3
{'Alex': 18, 'Bob': 10, 'Chris': 51, 'Dave': 25}
{'Alex': ['3H', '2H', 'AH', 'JH'], 'Bob': ['9H'], 'Chris': ['QS', '5H', '10H', 'KH', '4H'], 'Dave': ['6H', '7H', '8H', 'QH']}

ROUND #5
[['3S', '9H', 'QC', 'QS', '6S', '2H', '7D', '4C', 'AD', '6H', '10S', '2S', '6D'], ['10H', 'JC', '5D', 'JH', '8S', '5H', '4S', '10D', '7C', 'KS', '5S', '3D', '4D'], ['9D', '8D', 'KD', '5C', '8C', 'AS', 'QD', '7H', '8H', 'KC', '3H', '9S', '2D'], ['6C', 'JD', 'QH', '10C', '7S', 'AH', '9C', 'AC', '3C', '2C', 'JS', 'KH', '4H']]
['QC', 'JC', '5C', '2C'] QC 0
['3S', '8S', 'AS', '7S'] AS 2
['7D', '5D', '9D', 'JD'] JD 3
['4C', '7C', '8C', '6C'] 8C 2
['AD', '10D', '8D', 'QH'] AD 0
['9H', '10H', '7H', 'AH'] AH 3
['QS', 'JH', 'KC', '10C'] KC 2
['6D', '3D', 'KD', 'KH'] KD 2
['2H', '4D', 'QD', '4H'] QD 2
['6H', '5H', '8H', '9C'] 8H 2
['6S', '4S', '3H', 'AC'] 3H 2
['10S', 'KS', '9S', 'JS'] KS 1
['2S', '5S', '2D', '3C'] 5S 1
{'Alex': 19, 'Bob': 10, 'Chris': 72, 'Dave': 29}
{'Alex': ['QH'], 'Bob': [], 'Chris': ['QS', 'JH', 'KH', '2H', '4H', '6H', '5H', '8H', '3H'], 'Dave': ['9H', '10H', '7H', 'AH']}

ROUND #6
[['6H', '2S', '4C', '10D', '10H', '3S', 'QD', 'KD', '9H', '3D', '2D', '8S', '7H'], ['KH', 'AD', 'KS', '3C', 'JC', 'QH', 'QS', '4H', '9D', '5S', '9S', '9C', '4D'], ['5H', 'AS', '6D', '5C', 'AC', '10S', '8C', '10C', '6C', '8D', '7S', 'JD', '6S'], ['7C', 'AH', 'QC', '4S', 'JH', '5D', '8H', '2C', '7D', '3H', 'KC', '2H', 'JS']]
['4C', '3C', '5C', '2C'] 5C 2
['6H', 'KH', '5H', 'AH'] AH 3
['10H', 'JC', 'AC', '7C'] AC 2
['2S', 'KS', 'AS', '4S'] AS 2
['10D', 'AD', '6D', '5D'] AD 1
['9H', 'QH', '10S', 'JH'] QH 1
['3S', 'QS', '7S', 'JS'] QS 1
['7H', '4H', '8C', '8H'] 8H 3
['QD', '9C', '10C', 'QC'] QC 3
['KD', '9D', '8D', '7D'] KD 0
['3D', '4D', 'JD', '3H'] JD 2
['2D', '5S', '6C', 'KC'] KC 3
['8S', '9S', '6S', '2H'] 2H 3
{'Alex': 19, 'Bob': 26, 'Chris': 74, 'Dave': 37}
{'Alex': [], 'Bob': ['9H', 'QH', 'JH', 'QS'], 'Chris': ['10H', '3H'], 'Dave': ['6H', 'KH', '5H', 'AH', '7H', '4H', '8H', '2H']}


Chris just shot the moon!

ROUND #7
[['8D', '7H', '2D', '9C', 'QS', '7S', '10H', '6C', '9D', '4H', '10D', '6D', '4D'], ['10C', '8H', '10S', '8S', '5S', 'JD', '7C', 'AS', '9S', 'JC', 'JS', 'KC', '3H'], ['QD', 'JH', 'KH', '3C', 'KS', '6H', '6S', '9H', '4S', 'KD', '8C', 'AD', '3S'], ['AH', 'AC', '3D', 'QH', '7D', 'QC', '5C', '2H', '5D', '5H', '2C', '2S', '4C']]
['9C', '10C', '3C', '2C'] 10C 1
['7H', '8H', 'JH', 'AH'] AH 3
['6C', '7C', '8C', 'AC'] AC 3
['8D', 'JD', 'QD', '3D'] QD 2
['10H', '3H', 'KH', 'QH'] KH 2
['QS', '10S', 'KS', '2S'] KS 2
['4H', '8S', '6H', '2H'] 6H 2
['7S', '5S', '6S', '5H'] 7S 0
['2D', 'AS', 'KD', '7D'] KD 2
['9D', '9S', '9H', 'QC'] 9H 2
['10D', 'JS', '4S', '5C'] JS 1
['6D', 'JC', 'AD', '4C'] JC 1
['4D', 'KC', '3S', '5D'] KC 1
{'Alex': 20, 'Bob': 26, 'Chris': 95, 'Dave': 41}
{'Alex': ['5H'], 'Bob': [], 'Chris': ['10H', '3H', 'KH', 'QH', 'QS', '4H', '6H', '2H', '9H'], 'Dave': ['7H', '8H', 'JH', 'AH']}

ROUND #8
[['10C', 'KS', 'KC', '5C', 'QH', 'QS', 'AS', '2S', '8S', '7H', '8C', '10S', 'KD'], ['JC', '3C', '3H', '2C', 'AH', 'QC', '7S', '4C', '6H', 'JS', 'JD', '9H', 'AC'], ['2H', '10H', '4S', '3D', '2D', '5D', '7D', '6C', '4H', '9S', '8D', '6S', '8H'], ['9D', 'QD', '5S', '4D', '9C', '7C', 'JH', '10D', '3S', '5H', 'KH', 'AD', '6D']]
['10C', '2C', '6C', '9C'] 10C 0
['KS', '7S', '4S', '5S'] KS 0
['KC', 'JC', '2H', '7C'] KC 0
['5C', '3C', '10H', 'JH'] 5C 0
['QH', '3H', '4H', '5H'] QH 0
['QS', 'JS', '9S', '3S'] QS 0
['AS', 'AH', '6S', 'KH'] AS 0
['2S', '6H', '8H', '9D'] 2S 0
['8S', '9H', '3D', 'QD'] 8S 0
['7H', 'QC', '2D', '4D'] 7H 0
['8C', '4C', '5D', '10D'] 8C 0
['10S', 'JD', '7D', 'AD'] 10S 0
['KD', 'AC', '8D', '6D'] KD 0
{'Alex': 20, 'Bob': 52, 'Chris': 121, 'Dave': 67}
{'Alex': ['2H', '10H', 'JH', 'QH', '3H', '4H', '5H', 'QS', 'AH', 'KH', '6H', '8H', '9H', '7H'], 'Bob': [], 'Chris': [], 'Dave': []}

Alex just shot the moon!


{'Alex'} won with 46 points! {'Chris'} lost with a whopping 119 points...
The game lasted 9 rounds!
    • p4songer Avatar

      Hello again friend!

      Voile and I have been working through this for quite awhile now, and it seems that everything is up and running! The kata has been refactored quite a bit, but I implemented some of your suggestions as well as some of theirs! Let me know if you have any other issues!

  • Fbasham Avatar

    How sure are you of your implementation? I'm only asking because I'm consistently failing ~50 random tests every time I run the trainer, but pass everything else.

    Just so we're clear:

    • the winning player for each hand takes all of the played cards during that hand?
    • any round where one player doesn't win all of the point cards is considered a failed shooting the moon?
    • p4songer Avatar

      EDIT#2:

      • To your first question, yes. If a player wins a HAND with say, '2H', '3H', '2C', 'QS' they have scored 15 points on that hand.
      • To your second question, a player has shot the moon when they hold all point cards. For a naive application, this would normally add 26 points to the player's score total. Instead, they recieve zero points, and every other player recieves 26 points. So, a failure to shoot the moon means that someone, even just one other player, recieved a point in that round. This would add 25 to the "failed moon shooter" and 1 point to the point theif. Both other players would score zero. I should probably be clear that I don't have any code checking for a 'failed shooting the moon', and only have a check for 'shooting the moon'.

      To be honest, I'm not surprised the random tests might be broken. I was told in a previous kata to always put code related to test cases alongside the actual test case. This has made bug fixing very tedious and difficult to identify when I have to scan through 300 lines of code to see what's even being called where. Previously, I had put the random card dealer, and a few other variables into the preloaded section to isolate code that will only be used for randomness. This made bug fixing a lot easier, since I had more organized random tests, and could focus only on the code used for testing.

      The way random tests work right now is sub-optimal, but I'm not sure about how to fix it. I'm basically running a copy off of the random deal (the user is also given a copy of this) and running my code through a variable I named 'mySCORE', and then comparing the values in mySCORE to the values in SCORE. I worked through so many error messages during this process that I would have to guess on what the issue is. My best guess is that the issue is probably that you might be given a different list than the list my code is running on. Which would be a major issue... I'll take a look at it again with fresh eyes, but if I can't get this up and running by tonight, I'll probably un-publish until I have a way to fix it.

      All that being said, I would love your advice on how to keep the unit tests more organized. I loved the way the preloaded section worked, but it makes sense that I shouldn't allow the user to somehow access that part of the code. If you also have a better idea on how to compare user results from my results, that would also be great!

    • Fbasham Avatar

      Hehe, yeah I can understand how debugging would be painful. There's a chance I'm making mistakes too. I would change how you're doing the SCORE though. I don't see a good reason for this to be global state. Instead, make a function called play_game or something like that where you give us the initial hands of 13 cards for all players and then test us for the final score. We already know the stopping condition will be when a player scores over 100 points. I'll play around some more to see if this is a 'me' problem though.

    • p4songer Avatar

      Thanks for your help on this Kata, as well as the previous one! You're really giving me a run for my money when it comes to testing my code haha.

      I wanted to ask you what you meant about SCORE. Are you recommending removing this part of the test entirely? or just change the way it's being used in the random tests? I think it's probably still the best way to evaluate the actual scoring logic in the example tests, so my instinct is to keep it there for those tests at least. However, I do think you're right about changing the global nature of it in the random tests. This is where half of my errors were happening. I do like the idea of having you guys score the whole game on your end, and simply comparing the final results. As far as I can tell, all that this might require is re-writing the description, and fixing the test targets.

    • Fbasham Avatar

      Basically change your implementation to remove SCORE as a globally managed object. This means changing your code setup and how you're testing. You supply us with randomly drawn hands for the 4 players. Each play of the game (n rounds) will return the final score of the game. We know when to return because one of the players will have a score greater than 100.

      In code:

      def play_game(alex,brian,chris,dave):
  SCORE = {'alex':0,'brian':0,'chris':0,'dave':0
  
  ...
    our implementation
    
    the game is played for n rounds and stops when any player's score>100
  ...
  
  return SCORE
    • p4songer Avatar

      This is brilliant. I'm currently starting from scratch to get some better code to use in the tests generally, and also to weed out other weak spots. I will for sure work on something along these lines over the next few days. (maybe hours if I'm really lucky...)

    • Fbasham Avatar

      Ha, there's no rush. For now I'm going to put this on the back burner, or take a detailed look at one of the failed random tests to figure out if it's me. Go hand by hand and keep a good track of the scores. Who knows, maybe somebody else will solve this too.

  • Voile Avatar

    Please link part 1 and 2 in the description (since you're also asking us to copy our previous solutions to this kata).