[Tutor] Long post: Request comments on starting code and test code on chess rating project.

[Peter Otten]

boB Stepp wrote:

> I mentioned in one of the recent threads that I started that it is
> probably time for me to attempt a substantial project using the OO
> paradigm.  I have had no coursework or training in OOP other than my
> recent self-studies.  Everything (Other than toy practice examples.) I
> have coded to date has been strictly procedural, similar to what I
> learned in FORTRAN ages ago.  Also, I am continuing to try to do TDD
> and keep things DRY.  So I am hoping for a thorough critique of what I
> will shortly present.  Everything is fair game!  Hopefully my
> self-esteem is up to this critique!
> 
> The intent of this project is more than just calculate chess ratings.
> I also envision being able to store a record of all game results and
> player information for my school chess players that I give lessons to.
> Some functionality I will strive to include:  1) Be able to re-rate
> all games from a given date to the present.  2) Record demographic
> information about my players, such as names, current school year,
> current grade, etc.  3) Have a very friendly GUI so that when I have a
> finished project, a student could easily take over responsibility of
> maintaining the school's player ratings, if I so choose to allow this.
> 4) Having an editor where games and player data (But NOT ratings;
> these would have to be re-rated starting from a corrected game(s).)
> can be edited, inserted, or deleted with games (If game data is
> edited.) automatically be re-rated.  5) Et cetera ...
> 
> My current project structure is:
> 
> /Projects
>     /rating_calculator
>         /rating_calculator
>             __init__.py
>             main.py
>         /tests
>             __init__.py
>             test_main.py
> 
> I have started my coding with a RatingCalculator class.  The intent of
> this class is to gather all methods together needed to validate and
> calculate chess ratings.  My intent is to only ever have a single
> instance of this class existing during a given running of the program.

Why?

> (BTW, is there a technique to _guarantee_ that this is always true?)

Why? Would it harm one instance if there were another one? If so you have 
global state which usually runs counter to the idea of a class.

> I am only at the very beginning of coding this class.  I have not
> added any methods yet.  Currently I am adding class constants that
> will be used in this class' methods.  These constants are for internal
> class use only and should not be altered from outside the class.  

Still, a good unit test might be to initialise a RatingCalcultator with 
different "highest possible ratings" and then verify for each instance that 
actual ratings never exceed the specified value.

> Here
> is the current state of the main.py program (Which will probably be
> broken into multiple modules as the project develops.):
> 
> 
=================================================================================
> #!/usr/bin/env python3
> 
> """Module to process chess ratings."""
> 
> class RatingCalculator:
>     """This class contains methods to validate and calculate chess
>     ratings."""
> 
>     # Overestimated extremes for possible chess rating values.  Values
>     # outside this range should not be achievable.

If there are no hard limits, why give a limit at all?

> 
>     _LOWEST_POSSIBLE_RATING = 0

Again, this is just a number. Is it really worthwile explicit checking in a 
unit test? The most relevant tests address behaviour.

>     _HIGHEST_POSSIBLE_RATING = 3000
> 
>     # Fundamental constants for use in the chess rating formula.  The keys
>     # to these values are tuples giving the rating ranges for which these
>     # K-factors are valid.
> 
>     _K_MULTIPLIERS = {
>             (_LOWEST_POSSIBLE_RATING, 2099): 0.04,

The Python way uses half-open intervals. This has the advantage that there 
is a well-defined k-multiplier for a rating of 2099.5. Note that if you 
apply that modification you no longer need any upper limits. Use bisect (for 
only threee values linear search is OK, too) to find the interval from a 
list like [0, 2100, 2400], then look up the multiplier in a second list 
[0.04, 0.03, 0.02])

>             (2100, 2399): 0.03,
>             (2400, _HIGHEST_POSSIBLE_RATING): 0.02}
> 
>     _K_ADDERS = {
>             (_LOWEST_POSSIBLE_RATING, 2099): 16,
>             (2100, 2399): 12,
>             (2400, _HIGHEST_POSSIBLE_RATING): 8}
> 
=================================================================================
> 
> The test code in test_main.py is:
> 
> 
=================================================================================
> #!/usr/bin/env python3
> 
> """Module to test all functions and methods of main.py."""
> 
> import unittest
> import rating_calculator.main as main
> 
> class TestRatingCalculatorConstants(unittest.TestCase):
>     # Check that the constants in RatingCalculator have the proper values.
> 
>     def setUp(self):
>         # Create instance of RatingCalculator for use in the following
>         # tests. Create tuple of test-value pairs to interate over in
>         # subtests.
> 
>         self.rating_calculator = main.RatingCalculator()
>         self.test_value_pairs = (
>                 (self.rating_calculator._LOWEST_POSSIBLE_RATING, 0),

With the _ you are making these constants implementation details. Isn't one 
goal of unit tests to *allow* for changes in the implementation while 
keeping the public interface?

>                 (self.rating_calculator._HIGHEST_POSSIBLE_RATING, 3000),
>                 (self.rating_calculator._K_MULTIPLIERS[(
>                     self.rating_calculator._LOWEST_POSSIBLE_RATING,
>                     2099)], 0.04),
>                 (self.rating_calculator._K_MULTIPLIERS[(2100, 2399)],
>                 0.03), (self.rating_calculator._K_MULTIPLIERS[(2400,
>                     self.rating_calculator._HIGHEST_POSSIBLE_RATING)],
>                     0.02),
>                 (self.rating_calculator._K_ADDERS[(
>                     self.rating_calculator._LOWEST_POSSIBLE_RATING,
>                     2099)], 16),
>                 (self.rating_calculator._K_ADDERS[(2100, 2399)], 12),
>                 (self.rating_calculator._K_ADDERS[(2400,
>                     self.rating_calculator._HIGHEST_POSSIBLE_RATING)], 8))
> 
> 
>     def test_class_constants(self):
>         # Check that all class constants have the correct values.
> 
>         for tested_item, expected_value in self.test_value_pairs:
>             with self.subTest():
>                 self.assertEqual(tested_item, expected_value)
> 
> 
> if __name__ == '__main__':
>     unittest.main()
> 
=================================================================================
> 
> Instead of doc strings in the test code, I have used comments based on
> something I read online.  The point of doing this was so the doc
> strings would _not_ show up in the test run output.  Is this
> worthwhile advice to follow?

No. Why would you think it is? 

Also, it looks like only the first line of the test_xxx() methods' docstring 
is shown by default:

$ cat tmp.py
import unittest
import os

ok = bool(os.environ.get("OK"))

class A(unittest.TestCase):
    """NOT EVER SHOWN"""
    def test_foo(self):
        """
        HIDDEN.
        """
        self.assertTrue(ok)

    def test_bar(self):
        """SEEN.
        hidden
        hidden
        """
        self.assertTrue(ok)

if __name__ == "__main__":
    unittest.main()
$ python3 tmp.py 
FF
======================================================================
FAIL: test_bar (__main__.A)
SEEN.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tmp.py", line 19, in test_bar
    self.assertTrue(ok)
AssertionError: False is not true

======================================================================
FAIL: test_foo (__main__.A)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tmp.py", line 12, in test_foo
    self.assertTrue(ok)
AssertionError: False is not true

----------------------------------------------------------------------
Ran 2 tests in 0.001s

FAILED (failures=2)
$ 

> I'm looking forwards to everyone's thoughts.

Values are the most volatile and least important part of a program.
Start coding with the algorithms and tests covering those.