Skip to content

tournament: should reject inputs containing invalid lines #720

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

Closed
petertseng opened this issue Mar 13, 2017 · 3 comments
Closed

tournament: should reject inputs containing invalid lines #720

petertseng opened this issue Mar 13, 2017 · 3 comments

Comments

@petertseng
Copy link
Member

petertseng commented Mar 13, 2017

The tournament exercise currently asks the implementation to silently ignore invalid lines.

I would say it is not good design for an API to silently ignore invalid inputs, as that may produce surprising results for users of that API.

It seems more helpful to have inputs containing invalid lines to be rejected (in whatever way is appropriate for the target language).

My recommendation is:

  • change the description to state that input with invalid lines are rejected.
  • change the cases in the canonical data to expect errors for inputs with invalid lines.
    • I suggest that each test case contain only one invalid line, so that we can be sure that each individual invalid line is rejected. If a test case contained multiple invalid lines we might not be sure that an implementation would reject all invalid lines in that single case.
@rbasso
Copy link
Contributor

rbasso commented Mar 13, 2017

Considering that we just changed the test data in #717, adding a test case for invalid data, but also removing some more detailed information about the meaning of each type of invalid line, I am linking here the latest version of the file before the change, for convenience.

The information may be needed to generate the new test cases rejecting invalid data.

@petertseng petertseng changed the title tournament: consider rejecting inputs containing invalid lines tournament: should reject inputs containing invalid lines Apr 18, 2017
@Insti
Copy link
Contributor

Insti commented May 5, 2017

Alternatively, only have test cases that contain valid lines.

This exercise is about parsing a specified format and converting it into a pretty table, it is not a general string parsing problem.

@petertseng
Copy link
Member Author

I thought about it. No complaints here. I'll see if there are any before officially changing the recommendation in the issue title.

To be clear, to me:

"Do not test invalid inputs" is only slightly preferred to "Test that invalid inputs are rejected", and both of those are strongly preferred to "Test that invalid inputs are ignored".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants