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

word_count: test mixed case #234

Closed
behrtam opened this issue Nov 10, 2015 · 4 comments
Closed

word_count: test mixed case #234

behrtam opened this issue Nov 10, 2015 · 4 comments

Comments

@behrtam
Copy link
Contributor

behrtam commented Nov 10, 2015

I just saw a solution to the word_count exercise where my feeling was that this shouldn't pass all tests. There is already one mixed case test, but it has a specific word order so that this solution works.

def word_count(input):
    list = {}
    for word in input.split():
        if word in list:
            list[word]+=1
        elif word.lower() in list:
            list[word.lower()]+=1
        else:
            list[word]=1
    return list

>>> word_count('GO go Go')
{'GO': 1, 'go': 2}
>>> word_count('go Go GO')
{'go': 3}
>>>
@Dog
Copy link
Contributor

Dog commented Nov 10, 2015

The reason for this is that a change was applied to the test to normalize case.

0314cbf
#207

It was justified because other language tracks expect normalization - specifically ruby and clojure in the pull request. I also noticed Haskell does this normalization.

@behrtam
Copy link
Contributor Author

behrtam commented Nov 10, 2015

The problem is not that normalization is tested. The problem is also not that the test is not opinionated about how to normalize case.
The problem is that the test is so minimalistic that a user wrote code that passed the test suite while the solution only works if the function gets the words in the right order.

My simple solution to the mixed case test is to expand the test string "go Go GO" to
"go Go GO Stop stop" and assume as result a dict with the values 3, 2. That would still not force the user to only normalize to lower case.

def test_mixed_case(self):
    self.assertEqual([2, 3], sorted(list(word_count('go Go GO Stop stop').values())))

@kytrinyx
Copy link
Member

I actually don't mind which direction the user normalizes to, and I think that it would be nice to leave that up to the person solving the problem. I like this suggested approach and wouldn't mind changing the other tracks to use it.

@behrtam
Copy link
Contributor Author

behrtam commented Nov 12, 2015

Okay, did it for the Python track.

kytrinyx added a commit to exercism/problem-specifications that referenced this issue Nov 12, 2015
In exercism/python#234 we discussed how adding a second
word in the normalization case can let a track might allow
some languages to use sorted counts as a normalization-agnostic
assertion.
@behrtam behrtam closed this as completed Nov 18, 2015
masters3d pushed a commit to masters3d/x-common that referenced this issue Mar 12, 2016
In exercism/python#234 we discussed how adding a second
word in the normalization case can let a track might allow
some languages to use sorted counts as a normalization-agnostic
assertion.
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

No branches or pull requests

3 participants