Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

anagram: Change tests to be order-insensitive #392

Closed
rbasso opened this issue Oct 9, 2016 · 3 comments · Fixed by #393
Closed

anagram: Change tests to be order-insensitive #392

rbasso opened this issue Oct 9, 2016 · 3 comments · Fixed by #393

Comments

@rbasso
Copy link
Contributor

rbasso commented Oct 9, 2016

A user suggested that we could change anagram's test suite to check the output list ignoring the word's order.

The way it is now, anagramsFor word must behave as filter p to pass the tests.

The patch would be really simple,

sed -i 's/shouldBe/shouldMatchList/g' test/Tests.hs

... but first we have to decide if it is important to check the output order.

Does anyone have an opinion on this?

@rbasso rbasso added the question label Oct 9, 2016
@petertseng
Copy link
Member

Strangely, I had never thought of this. Probably many other languages check the same ordering too!

It seems OK to use shouldMatchList, if all we are asking in the problem is "which of these words are anagrams?"

I am sure there are various arguments for why it makes sense to keep the output in the same order as the input (and my implementations will probably keep it so) but I don't think it is essential to the problem that it has to be that way.

@rbasso
Copy link
Contributor Author

rbasso commented Oct 9, 2016

Agreed! The order doesn't seems to be an essential part of the problem.

I marked this issue as a good first patch to see if any new contributor is willing to open a PR.

@abo64
Copy link
Contributor

abo64 commented Oct 11, 2016

I agree, too. Have the test suite as general as possible to allow for as many solutions as possible. In the end we want to explore those possible solutions for the maximum learning effect and not be arbitrarily restricted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants