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: Make the test suite as flexible as possible #393

Merged
merged 1 commit into from
Oct 12, 2016
Merged

anagram: Make the test suite as flexible as possible #393

merged 1 commit into from
Oct 12, 2016

Conversation

rbasso
Copy link
Contributor

@rbasso rbasso commented Oct 10, 2016

For a while, we are doings things that seem to increase the number of solutions accepted by the test suites:

All of those things where based on the same idea: To allow more diverse solutions.

This seems to be aligned to what was discussed in exercism/discussions#41 and exercism/discussions#44.

Taking this process to the next level, if the essence of the anagram problem is to find anagrams, there is no reason to restrict the solutions to work only on Strings and Lists. The following signatures seem equally reasonable:

anagramsFor :: String ->          [String] ->          [String]
anagramsFor :: String -> Sequence  String  -> Sequence  String
anagramsFor :: String -> Set       String  -> Set       String
anagramsFor :: Text   ->          [Text]   ->          [Text]
anagramsFor :: Text   -> Sequence  Text    -> Sequence  Text
anagramsFor :: Text   -> Set       Text    -> Set       Text

This PR is an experiment on trying to write the most general possible test suite, allowing users to create beautiful solutions using important libraries that are usually not allowed in most of the exercises.

Closes #392.

. toList
. anagramsFor (fromList subject)
. fromList
. map fromList
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note for my study - this allows either Text or String to be used. Interesting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IsList is fantastic. Way more flexible than I imagined. 😄

@petertseng
Copy link
Member

Seems good! I hope the many fromList and toList do not make it too hard for students to understand what is going on. That will depend on having their feedback so it seems like it is time to merge this.

@petertseng petertseng merged commit 767b32e into exercism:master Oct 12, 2016
@rbasso rbasso deleted the anagram-generalize-test-suite branch October 12, 2016 03:26
@petertseng
Copy link
Member

I manually verified that this works on also String -> Set String -> Set String and String -> [String] -> [String]. I kind of wish that validation could be performed automatically. For example, if we would have multiple example files, and our travis.yml would test each one of them in turn.

@rbasso
Copy link
Contributor Author

rbasso commented Oct 12, 2016

Hummm...this is a great idea, @petertseng.

I was thinking in something similar to solve the problem of having a separated package.yaml for the example. I think that maybe having multiple examples would automatically solve that.

I'll try to think in a way to test multiple solutions on Travis.

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

Successfully merging this pull request may close these issues.

anagram: Change tests to be order-insensitive
2 participants