exercism / python

Exercism exercises in Python.
https://exercism.org/tracks/python
MIT License
1.87k stars 1.26k forks source link

Allergies' "test_allergic_to_everything" implicitly requires a specific list ordering #186

Closed spthm closed 9 years ago

spthm commented 9 years ago
def test_allergic_to_everything(self):
    self.assertEqual(
        ('eggs peanuts shellfish strawberries tomatoes '
         'chocolate pollen cats').split(),
        Allergies(255).list)

This requires that Allergies(255).list return items in exactly the same order as they are written above. If the order is not important, assertItemsEqual() is better. Unfortunately, in Python 3 this was renamed to assertCountEqual(), which does not exist in Python 2.7. So perhaps assertEqual(sorted(expected), sorted(actual)) would in fact be the best solution here.

Alternatively, if the requirement for identical ordering is intentional, it should be made clearer using assertSequenceEqual(). At present, the README merely states that it should return "All the allergens Tom is allergic to."

I believe this may also affect other Python exercises.

I am happy to submit a pull request for said changes, but would first like to know if there is any consensus on exactly what the correct behaviour should be: to require identical ordering or not?

Dog commented 9 years ago

Order isn't important in the result, though the order does in a way give a hint it is related to binary. Feel free to do a pull request, because I think it would be better to not force order.

We can handle the change between Python 2 and 3 without too much of an issue. Using a library like six would make it easier, but it would probably be difficult to make people install it.

I think we would could do something along the lines of:

if sys.version_info < (3,):
      assertItemsEqual()
else:
      assertCountEqual()

for the time being.

ZacharyRSmith commented 9 years ago

I assertEqual(sorted(expected), sorted(actual)) in commit 7ba9220a55d626d0211f3ee9c5beedd47dddf4c4 of pull request #194

kytrinyx commented 9 years ago

That works for me, thanks!