Sort results for `list` in `allergies` tests

exercism / unison

Exercism exercises in Unison.

https://exercism.org/tracks/unison

MIT License

3 stars 19 forks source link

Sort results for `list` in `allergies` tests #45

Closed brianloveswords closed 2 years ago

brianloveswords commented 2 years ago

My first solution for this problem ended up failing when I produced the right results, but in the wrong order!

If I think about the problem domain, I'm not sure the ordering of the list of allergens is important—["eggs", "peanuts"] and ["peanuts", "eggs"] represent semantically equivalent answers to the question "what set of allergies does this score represent?"

Sorting the results in the tests will open this up to more correct (IMO) solutions, so I put together this PR that does that!

I ran bin/test and everything seemed to go (but I'm not sure I understand the output), and I also tested locally against runarorama's solution and stew's solution to make sure they still pass and they do.

ErikSchierboom commented 2 years ago

An alternative would be to use sets instead of lists.

brianloveswords commented 2 years ago

@ErikSchierboom I considered that but ended up going with the sorted approach for two reasons:

if I changed the type signature for list to produce a Set Text instead of a [Text], I was concerned it would break all of the community implementations.
if I changed the tests to convert the results to a Set Text, I was concerned that it would accept inputs that are maybe not quite valid—for examples ["eggs", "eggs", "eggs", "eggs"] is probably something weird to produce and probably should fail, I think.

That said, if I were modeling this from scratch, I would probably use Set Text (or more likely, Set Allergen baed on a unique type)!

rlmark commented 2 years ago

Thank you @brianloveswords!!! Good catch! I'll merge this and we can also consider if we want to use a different data type for the exercise altogether. The existing submissions won't be affected by our changes (they'll still be listed as having been passed).

ErikSchierboom commented 2 years ago

If I changed the tests to convert the results to a Set Text, I was concerned that it would accept inputs that are maybe not quite valid

I can see that argument yes.

The existing submissions won't be affected by our changes (they'll still be listed as having been passed).

That's not exactly true, as by default we also test solutions against updated versions of the exercise. The community solutions page by default only shows solutions passing the latest version of the tests, so this would affect them.

rlmark commented 2 years ago

Oh gotcha @ErikSchierboom, thats good to know!