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

all-your-base: Be explicit about error cases #1000

Merged
merged 1 commit into from
Nov 11, 2017
Merged

all-your-base: Be explicit about error cases #1000

merged 1 commit into from
Nov 11, 2017

Conversation

petertseng
Copy link
Member

@petertseng petertseng commented Nov 10, 2017

all-your-base 2.0.0

This uses the error object defined in
#401 (comment)
and thenceforth added to the README's Test Data Format.

In #280 we made
the explicit choice to leave various cases expected: null where tracks
can make their own decisions (leading zeroes, the representations of
zero, whether empty sequence is acceptable as input).

However, since expected: null is also used for cases where there is
always an error (and there is no decision being left to the tracks),
it becomes unreasonably hard to tell cases in these two categories
apart.

To make this easier, all cases in the latter category (always are
errors) can canonically be marked as such.

},
{
"description": "negative digit",
"property": "rebase",
"input_base": 2,
"input_digits": [1, -1, 1, 0, 1, 0],
"output_base": 10,
"expected": null
"expected": {"error": "all digits must satisfy 0 <= d < input base"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pedantic_mode: Is -1 really a digit?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see why one would ask this question, and it is true that in all bases allowed in this problem, digits can be considered to be unsigned values.

So thus, I present for academic curiosity https://en.wikipedia.org/wiki/Balanced_ternary, in which there is the possibility of negative digits.

@petertseng
Copy link
Member Author

Wait a second. I think I can now remove "4. How should invalid input be handled?" from the list of unanswered questions at the top of the file now, right?

@petertseng
Copy link
Member Author

So, like this then?

If not, Remind me to go back to 7983e19

@Insti
Copy link
Contributor

Insti commented Nov 10, 2017

I'm becoming more convinced that invalid input handling should not be present in the canonical data. See also: #902

@petertseng
Copy link
Member Author

I wonder if I find it funny that removing all these cases would move to version 1.2.0, whereas keeping them moves to version 2.0.0.

Regardless of that, I'll think about whether I want to remove them.

@Insti
Copy link
Contributor

Insti commented Nov 10, 2017

That's more an indication of how screwed up our versioning system is.

all-your-base 2.0.0

This uses the error object defined in
#401 (comment)
and thenceforth added to the README's **Test Data Format**.

In #280 we made
the explicit choice to leave various cases `expected: null` where tracks
can make their own decisions (leading zeroes, the representations of
zero, whether empty sequence is acceptable as input).

However, since `expected: null` is also used for cases where there is
always an error (and there is no decision being left to the tracks),
it becomes unreasonably hard to tell cases in these two categories
apart.

To make this easier, all cases in the latter category (always are
errors) can canonically be marked as such.
@petertseng
Copy link
Member Author

What I will say today, and that I can reasonably say there is agreement on, is that if the invalid inputs should be tested, we agree that it is better to expect an error rather than null.

The merging of this PR is not to be taken as an acceptance or rejection of the statement "we should remove the invalid inputs from the canonical data"

@petertseng petertseng merged commit 5470fef into exercism:master Nov 11, 2017
@petertseng petertseng deleted the ayb-error branch November 11, 2017 08:07
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.

3 participants