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

Remove deprecated HashSet from allergies tests #209

Closed
devonestes opened this issue Aug 1, 2016 · 2 comments · Fixed by #210
Closed

Remove deprecated HashSet from allergies tests #209

devonestes opened this issue Aug 1, 2016 · 2 comments · Fixed by #210

Comments

@devonestes
Copy link
Contributor

I was just looking through the tests for another thing I'm doing, and I saw that we're still using HashSet in allergies here: https://github.com/exercism/xelixir/blob/master/exercises/allergies/allergies_test.exs#L75

That's now deprecated, so we'll have to update it to use MapSet. I'll probably take care of that today, but I wanted to write it down so I don't forget (or so someone else can take care of it).

@martinsvalin
Copy link
Contributor

I had a quick look, as I didn't recall this exercise had anything to do with sets. It seems we're using sets to ignore order in the lists. Seems simpler to have the helper be something like

def assert_same_elements(actual, expected) do
  assert Enum.sort(actual) == Enum.sort(expected),
         "Expected #{inspect actual} to have the same elements as #{inspect expected}"
end

@devonestes
Copy link
Contributor Author

Yeah, I guess technically that's correct and we could do that. However, I do like to see some of the less frequently used parts of Elixir used occasionally. After going through all the exercises over the last couple of days I've noticed that someone can make their way through the entirety of the Elixir track on here and not even touch a good chunk of the language. Things like this I think might be helpful in exposing users to new things to check out - in this case, MapSet.

However, since there is a simpler way of expressing this, exposing these language features in this way might be counterproductive. I'm totally open to discussion from others on if we should refactor that in the way you propose, but I think a good first step is just getting rid of the deprecation without refactoring since we know for sure that's gotta go.

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

Successfully merging a pull request may close this issue.

2 participants