Put calculations in right place

Move the calculations out of the `pre_check()` function.
Also do smarter checks:
  - If len(sequence) == 1, we don't need any rolls. In this case, just
    return sequence[0]
  - If len(sequence) is a factor of dice_sides, we only need one roll
    along with the modulo operator.
  - For everything else, just require one extra roll, bringing it to a
    total of 2 rolls. This should remove a little bit of the edge bias
    introduced by the modulo
Run the `pre_check()` function after the calculations have been done.

For the unit tests:
  - remove the function `test_choice_len_too_short()`;
    we no longer have to raise an exception when the sequence length is
    too short.
  - Write new tests for the cases where `dice_sides = 6` and `len(sequence)`:
     - 1
     - 2,3
     - 4,5

diceware/random_sources.py

@@ -131,15 +131,8 @@ class RealDiceRandomSource(object):
         We make sure that `num_rolls`, the number of rolls, is in an
         acceptable range and issue an hint about the procedure.
         """
-        use_modulo=False
         if num_rolls == 0:
            raise(ValueError)
-        if num_rolls < 1:
-           # If this happens, there are less values in the sequence to choose from than there are dice sides.
-           # We will generate a large integer (larger than 100*len(sequence)),
-           # and use modulo to select from it
-           num_rolls = int(math.ceil(math.log(100*len(sequence), self.dice_sides)))
-           use_modulo=True
         if (self.dice_sides ** num_rolls) < len(sequence):
             print(
                 "Warning: entropy is reduced! Using only first %s of %s "
@@ -150,13 +143,27 @@ class RealDiceRandomSource(object):
         print(
             "Please roll %s dice (or a single dice %s times)." % (
                 num_rolls, num_rolls))
-        return num_rolls, use_modulo
+        return
 
     def choice(self, sequence):
         """Pick one item out of `sequence`.
         """
         num_rolls = int(math.log(len(sequence), self.dice_sides))
-        num_rolls, use_modulo = self.pre_check(num_rolls, sequence)
+        use_modulo = False
+        if num_rolls < 1:
+           # If this happens, there are less values in the sequence to choose from than there are dice sides.
+           # First check whehter the length is 1. Then we don't have to do anything else
+           if len(sequence) == 1:
+           # Check whether len(sequence) is a factor of dice.sides
+              return sequence[0]
+           if self.dice_sides % len(sequence) == 0:
+              use_modulo = True
+              num_rolls = 1
+           else:
+              # otherwise We will perform one extra roll and apply modulo
+              use_modulo=True
+              num_rolls = 2
+        self.pre_check(num_rolls, sequence)
         result = 0
         for i in range(num_rolls, 0, -1):
             rolled = None

tests/test_random_sources.py

@@ -219,13 +219,6 @@ class TestRealDiceRandomSource(object):
         src = RealDiceRandomSource(None)
         assert src.choice([1, 2, 3, 4, 5, 6]) == 1
 
-    def test_choice_len_too_short(self, monkeypatch):
-        # We raise an exception if choice gets less than 6 elements.
-        self.fake_input_values(["1"], monkeypatch)
-        src = RealDiceRandomSource(None)
-        with pytest.raises(ValueError):
-            assert src.choice([1, 2, 3, 4, 5])  # list len < 6
-
     def test_choice_input_lower_value_borders(self, monkeypatch):
         # choice() does not accept "0" but it accepts "1"
         self.fake_input_values(["0", "1"], monkeypatch)
@@ -264,16 +257,27 @@ class TestRealDiceRandomSource(object):
         out, err = capsys.readouterr()
         assert "Please roll 5 dice (or a single dice 5 times)." in out
 
-    def test_sequence_less_than_dice_sides(self, capsys):
+    def test_sequence_less_than_dice_sides(self, capsys, monkeypatch):
         # Test to see whether we can use a n-sided die to choose from a sequence with less than n items
         src = RealDiceRandomSource(None)
-        # If we give a value of num_rolls < 1,
-        # the pre_check should fix the number of rolls
-        # and notify us to use the modulo
-        num_rolls, usemodulo = src.pre_check(0.5, ['abc'])
-        assert num_rolls > 1
-        assert usemodulo
-        # But if we give num_rolls > 1, it should do no such thing:
-        num_rolls, usemodulo = src.pre_check(5, range(1, 6 ** 5))
-        assert num_rolls == 5
-        assert not usemodulo
+        src.dice_sides = 6
+        # A length of 1 requires no rolls
+        self.fake_input_values(["1"], monkeypatch)
+        picked = src.choice([1])
+        out, err = capsys.readouterr()
+        assert "roll" not in out
+        assert picked == 1
+        # A length of 2,3 only requires 1 roll
+        for choice_length in (2,3):
+            self.fake_input_values(["1"], monkeypatch)
+            picked = src.choice(range(1,choice_length + 1))
+            out, err = capsys.readouterr()
+            assert "roll 1 dice" in out
+            assert picked == 1
+        # A length of 4,5 requires 2 rolls
+        for choice_length in (4,5):
+            self.fake_input_values(["1","1"], monkeypatch)
+            picked = src.choice(range(1,choice_length + 1))
+            out, err = capsys.readouterr()
+            assert "roll 2 dice" in out
+            assert picked == 1