-
-
Notifications
You must be signed in to change notification settings - Fork 543
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: remove punctuation or treat as error? #2033
Comments
The problem-specifications does not dictate how the tests end up in the tracks, and some tracks may error, others may report, while still other tracks may not even implement some of the tests. In the end, it is likely valid to ignore all things other than the digits themselves. If we collect the digits and concern ourselves with only the digits, we actually need to worry about quite a bit less. Everything else (which is a lot more numerous than just the 10 digits) becomes a non-concern if we solve in this manner. What I am saying, like in real life, sometimes extra information is there to try to help, but can still get in the way. Still, the tracks are allowed to take the canonical information and choose to put it in place as they see fit. It may not be allowed, or permitted, but may also be ignored. It surely is not permitted in the cleaned up phone number, but the track may choose to use the test, ignore the test, raise an exception or report the problem, among perhaps other choices. As far as the NANP, there are no I hope that helps. The README is a general "BOSS" kind of statement, while the tests themselves end up as a clarification to ambiguities (hopefully) that may be in the README, at least that is how I manage this. They should not be contradictory, but can be educational in nature to give a less specified README, and an enforcing (but not too enforcing, or over specified) test file. |
I know how that works, I've been a mentor for quite some time now and I've translated some exercises. But maybe I don't understand what this test is about or the role of the Let me phrase it differently. This is the test : {
"uuid": "065f6363-8394-4759-b080-e6c8c351dd1f",
"reimplements": "4bd97d90-52fd-45d3-b0db-06ab95b1244e",
"description": "invalid with punctuations",
"comments": [
"make input invalid only due to punctuation; area code may not start with 1"
],
"property": "clean",
"input": {
"phrase": "523-@:!-7890"
},
"expected": {
"error": "punctuations not permitted"
}
}, What is the intent of that test? If it's about the number of digits then we already have such a tests: invalid when 9 digits. When I read the name ("invalid with punctuations"), the comment ("make input invalid only due to punctuation; area code may not start with 1"), and the expected error message ("punctuations not permitted") I get the impression that this test wants to disallow some punctuation. If my interpretation is valid it contradicts the instructions ("removing punctuation"). If I misread that (disclaimer: I'm not a native English speaker) then maybe the wording could be improved or another test could be added with a valid number that contains that kind of punctuation. Or maybe I misunderstood the "error" property: What is it supposed to mean other than what kind of error is expected? |
I think @astrieanna might have some insight on this specifically. It surely needs some tender loving care if it is to exist, and in my opinion should go away, because it focuses on "noise" and we do not care about noise. We care about the digits. And this exercise is about cleaning. |
The instructions say that the "task is to clean up differently formatted telephone numbers by removing punctuation [...]".
But the test invalid with punctuations (and its predecessor) seem to imply that certain punctuation characters are forbidden, especially since it wants the error "punctuations not permitted".
Frankly, I'm not sure what the exercise wants: Is there a list of allowed punctuation characters like '+-()' or a list of disallowed punctuation like '@:!'?
Personally I would either list the allowed punctuation characters in the instructions or in one of the tests explicitly, or deprecate that test and just expect the solutions to remove all punctuation characters without error.
The text was updated successfully, but these errors were encountered: