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

Updating tests for phone-number #1032

Merged
merged 2 commits into from
Jan 9, 2025

Conversation

jagdish-15
Copy link
Contributor

Pull Request: Syncing Tests for Phone-Number Exercise

I've synced the toml file with the problem specification repository. Currently, the tests are functioning as expected, with the program returning a "0000...0" phone number for invalid inputs. However, according to the canonical-data.json file in the problem-specification repository, the suggested behavior is for the program to throw an error when an invalid phone number is passed as an argument.

To align with the problem specification, we would need to:

  1. Add the error handling where required in the test file.
  2. Update the example answers accordingly.

Since the program is currently working with the existing approach (returning "0000...0"), I'm asking whether you'd prefer to proceed with the new specification of throwing an error for invalid phone numbers. Let me know how you'd like to proceed.

@wolf99
Copy link
Contributor

wolf99 commented Jan 9, 2025

Thank you for all the updates @jagdish-15 !

Without thinking this through very deeply, I feel like changing this to add more complex error handling would result in changing the FUT interface such that the exercise becomes more about string memory management rather than string interpretation and mutation.

For that reason I would err on the side of keeping the existing behavior for errors.

I'm open to correction though!

@jagdish-15
Copy link
Contributor Author

You're welcome, @wolf99! I completely agree with your observation. Keeping the approach consistent with exercises like protein-translation is a great choice, as introducing complex errors could unnecessarily complicate things without adding much value.

I've updated all the necessary files accordingly, and since no changes are needed to the test file, everything should be good to go. Let me know if there's anything else you'd like me to address! 😊

@wolf99 wolf99 merged commit e30aae9 into exercism:main Jan 9, 2025
4 checks passed
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

Successfully merging this pull request may close these issues.

2 participants