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: remove punctuation or treat as error? #2033

Open
siebenschlaefer opened this issue May 6, 2022 · 3 comments
Open

phone-number: remove punctuation or treat as error? #2033

siebenschlaefer opened this issue May 6, 2022 · 3 comments

Comments

@siebenschlaefer
Copy link
Contributor

siebenschlaefer commented May 6, 2022

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.

@kotp
Copy link
Member

kotp commented May 10, 2022

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 + or - or () characters. The + is only there when typeset in paper form (old days) and still used on displays today, while also being handy to separate the 4 parts of the North American Number Plan for humans to read.

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.

@siebenschlaefer
Copy link
Contributor Author

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.

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 error property.

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?

@kotp
Copy link
Member

kotp commented May 11, 2022

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.

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

2 participants