search

emilybache / Tennis-Refactoring-Kata

This is a Refactoring Kata based on the rules of Tennis
https://youtu.be/XifUs1FhWRc
MIT License
737 stars 1.27k forks source link

remove hard-coding of "player1" and "player2" to make the Java code correct before starting refactoring #108

Closed ivanmoore closed 1 year ago

ivanmoore commented 1 year ago

This is for the Java code only.

Three of the TennisGame implementations use hardcoded Strings "player1" and "player2" to check which player is playing. This means that those classes only give the correct results if the player names happen to be "player1" and "player2" - which they are in the tests but I believe it is not the intention that only those names should give the correct results.

This change is to make those implementations use the player names which the TennisGame was created with, so they work correctly with any player names.

The test has been updated to introduce randomness in the player names in order to show that the scoring works correctly regardless of the names. The randomness does not introduce any non-determinism.

emilybache commented 1 year ago

I understand why you wanted to fix this. Unfortunately I didn't want it fixed :-) This is supposed to be legacy code. The player name problem is deliberate.

I hope you had fun working on this exercise in any case. Thankyou for taking the time to send a pull request.

ivanmoore commented 1 year ago

No worries. Sorry I reached the conclusion that it was intended to be a pure refactoring exercise because the README says "You should not need to change the tests", and version number 4 of the Java and Kotlin ones doesn't have this bug, and none of the python versions do. Should they all be consistently wrong and maybe should "You should not need to change the tests" be removed from the README?

emilybache commented 1 year ago

good point about the README. I'll have to check the python version