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

phone-number: Does not properly specifiy test for numbers with length greater than 11 #608

Closed
wolf99 opened this issue Feb 22, 2017 · 5 comments

Comments

@wolf99
Copy link
Contributor

wolf99 commented Feb 22, 2017

The specification for the phone-number exercise says

  • If the phone number is more than 11 digits assume that it is a bad number

But the phone-number/canonical-data.json does not specify checking this properly. It checks for length of 11 without leading double 1 to be invalid and for length of 12 to be invalid. It should instead (according to the brief in the readme)check for any number greater than 11 to be invalid.

@tejasbubane
Copy link
Member

tejasbubane commented Mar 10, 2017

@wolf99 If I understand correctly what you mean, the test for length 12 should be enough checking the case of greater than 11. Or maybe we need just one more testcase of arbitrary length greater than 11. What do others think? cc @rbasso @kotp

@rbasso
Copy link
Contributor

rbasso commented Mar 10, 2017

@wolf99, maybe I got the idea, but I'm not sure. Could you clarify what exactly you want to be changed, preferable with the old and new json fragments?

@wolf99
Copy link
Contributor Author

wolf99 commented Mar 10, 2017

Current

        {
          "description": "invalid when 12 digits",
          "property": "clean",
          "phrase": "321234567890",
          "expected": null
        },

New

        {
          "description": "invalid when more than 11 digits",
          "property": "clean",
          "phrase": "321234567890",
          "expected": null
        },

The old version means that tests will check for implementations to use an input check similar to

   if (length == 12) {
       return ERROR;
   }

When in fact it should be

   if (length > 11) {
       return ERROR;
   }

I.E: All lengths greater than 11 are invalid, not just lengths of 12.

@tejasbubane
Copy link
Member

All lengths greater than 11 are invalid, not just lengths of 12.

Got it 😃 Nice catch! Would you mind sending a PR for this? Or else I can do the change.

@wolf99
Copy link
Contributor Author

wolf99 commented Mar 12, 2017

Added #719 then

emcoding pushed a commit that referenced this issue Nov 19, 2018
generator: remove premature update to README
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