The instructions for the phone-number problem do not contain any description of the negative test cases. This leads to the situation where following only the instructions document will produce code that passes all the positive test cases pass (that is, all the test cases where phone number is cleaned up and returned), but all the negative test cases fail (that is, test cases that assert that null should be returned). As there is no description of why the function would ever return null, and no description of what constitutes invalid, (the only use of the word “invalid” is “you’ll identify and filter out any invalid entries”), the only option is to go through the tests, one by one, and add exclusions and counter exclusions.
This feels wrong, like half the specification is missing.
I propose that the following negative conditions are added (with final wording to be determined if accepted):
a phone number with fewer than 10 digits (after cleaning) is invalid
a phone number with more than 11 digits (after cleaning) is invalid
a phone number where the area code starts with a 0 or a 1 is invalid
a phone number where the exchange code starts with a 0 or a 1 is invalid
a phone number where the country code exists and is not 1 is invalid
You are correct that the instructions file does not contain the full specs. This is intentional; Exercism takes a Test Driven Development approach where you are expected to run the tests to fully understand the requirements.
I’m not convinced a change is warranted here. Each of these conditions are already covered in the instructions (see below). Their significance might not be clear when you first jump in, but we’d be repeating ourselves if this text is added. We’re also missing some other conditions from the tests like you can’t use letters or punctuations. Would they be added as well?
a phone number with fewer than 10 digits (after cleaning) is invalid
NANP numbers are ten-digit numbers…
a phone number with more than 11 digits (after cleaning) is invalid
NANP numbers are ten-digit numbers … Sometimes they also have the country code (represented as 1 or +1 ) prefixed.
a phone number where the area code starts with a 0 or a 1 is invalid
NXX NXX-XXXX where N is any digit from 2 through 9 and X is any digit from 0 through 9
a phone number where the exchange code starts with a 0 or a 1 is invalid
NXX NXX-XXXX where N is any digit from 2 through 9 and X is any digit from 0 through 9
a phone number where the country code exists and is not 1 is invalid
The PR that this is in reference to is likely zig#567.
I believe that the link was not followed to start the conversation in the Zig track, but I believe this is the case where an append.md file addition could be made to talk about the negative case, if warranted by the Zig maintainer(s).
I believe that the Phone Numbers description overall is good, the way to address the “not good” path is language dependent and should not be covered at this level, other than in general terms.
Thanks for the context. That linked PR was superseded by phone-number: return null on invalid input by keiravillekode · Pull Request #568 · exercism/zig · GitHub which is proposing a track-specific append rather than updating the instructions in place. So that should be sewn up nicely. It looks like there’s a precedent for documenting the return type for invalid phone numbers so I’ll take another look at my tracks’ implementations and see if it makes sense to do that.
Thanks @BNAndras, I’m ok with leaving things as they are since all the negative cases are at least match up to something in the instructions. I think the comparison to letters and other non-digits is a good point, there’s no need to add an explicit statement to disallow letters, so I how by the same token you could say the same about the other requirements.