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

Exercises with non-standard error indicators #1311

Closed
11 of 13 tasks
Insti opened this issue Aug 30, 2018 · 10 comments
Closed
11 of 13 tasks

Exercises with non-standard error indicators #1311

Insti opened this issue Aug 30, 2018 · 10 comments

Comments

@Insti
Copy link
Contributor

Insti commented Aug 30, 2018

There are several exercises with non-standard error indicators.

The canonical data documentation says

If an error is expected (because the input is invalid, or any other reason), the value at "expected" should be an object containing exactly one property, "error", whose value is a string.

I started creating issues for these but found there were a few more, so I've grouped them all together here.

Todo:

Update the canonical-data.json for each exercise to use the correct error format.

Exercises:

See also: hamming canonical-data.json for another example of how to do things the right way.

@petertseng
Copy link
Member

petertseng commented Aug 30, 2018

Anything in https://github.com/petertseng/exercism-problem-specifications that has to pass a custom error to verify must be using a non-standard error (Since the default error is the standard).

So, that'd be:

$ git grep 'error: ->' | cut -d/ -f2            (08-30 19:35)
binary
change
forth
grains
largest-series-product
ocr-numbers
pascals-triangle
say
trinary
variable-length-quantity
wordy

Note that that is a sufficient condition but not a necessary condition, so the omission of any exercise in this list does not give it a free pass (particularly, please still do pay attention to phone-number and queen-attack even though they are not in my list but are in the list in the issue description)

@rpottsoh
Copy link
Member

rpottsoh commented Aug 30, 2018

Is #902 worth considering again? Do all these exercises need to be testing for invalid input? Is looking for invalid input germane to the exercise, if not can those cases be removed from their respective exercises?

@Insti
Copy link
Contributor Author

Insti commented Aug 30, 2018

@petertseng nice, you've picked up some I didn't find (probably because the Ruby track doesn't have generators for them.)

@rpottsoh probably. I'm strongly in favor of removing invalid input related tests.

@rpottsoh
Copy link
Member

binary and trinary are deprecated, still should be considered?

rpottsoh added a commit to rpottsoh/exercism-problem-specifications that referenced this issue Sep 21, 2018
per exercism#1311 proper error handling object incorporated into last two test cases.
rpottsoh added a commit to rpottsoh/exercism-problem-specifications that referenced this issue Sep 21, 2018
rpottsoh added a commit to rpottsoh/exercism-problem-specifications that referenced this issue Sep 21, 2018
@ErikSchierboom
Copy link
Member

binary and trinary are deprecated, still should be considered?

I wouldn't mind them not having been updated to be honest. As they are deprecated, generators should be ignoring them I think.

petertseng added a commit that referenced this issue Sep 24, 2018
change 1.3.0

As proposed in
#1313
#905 (comment)

In contrast to the proposal in
#336 (comment)

Although -1 is a sentinel value, the sentinel value had been overloaded
in this JSON file to mean two separate conditions:

* "Can't make target" (ostensibly, we might be able to make the target
  with a different set of coins)
* "Target is negative" (no matter what coins we provide, this target is
  always invalid)

To make clear that these two conditions are different, we use an error
object describing each.

This error object was defined in
#401

Note that this commit is not a decree that all languages must represent
these conditions as errors; languages should continue to represent the
conditions in the usual way for that language. It is simply a
declaration that these two conditions bear enough consideration that
we'll represent them with a different type and also clearly
differentiate between the two.

Closes #1313
Checks the related box in #1311
petertseng added a commit that referenced this issue Sep 24, 2018
binary-search 1.2.0

As discussed in
#1312

Although -1 is a sentinel value, using this sentinel value is not the
usual course of action in some languages. In using an error object, we
avoid giving the wrong idea that we are requiring the use of the
sentinel value.

This error value was defined in
#401

Of course, languages that wish to use a sentinel value may continue to
do so; this commit is not intended to decree that sentinel values are
forbidden.

Neither is this commit decreeing that all languages must represent this
condition as an error; it is simply a declaration that this condition
bears enough consideration that we'll represent it with a different
type.

Closes #1312
Checks the related box in #1311
rpottsoh added a commit to rpottsoh/exercism-problem-specifications that referenced this issue Sep 24, 2018
per exercism#1311
pascals-triangle: update comment re: errors

Per @Insti's suggestion....
rpottsoh added a commit to rpottsoh/exercism-problem-specifications that referenced this issue Sep 24, 2018
per exercism#1311 proper error handling object incorporated into last two test cases.
wordy: lowerCased error message 2nd word

per @ErikSchierboom suggestion.
wordy: reworded comment to indicate 'error'

per @Insti's request.
wordy: error messages more helpful.
rpottsoh added a commit to rpottsoh/exercism-problem-specifications that referenced this issue Sep 24, 2018
per exercism#1311
ocr-numbers: remove redundant comment

per @Insti's suggestion.
ocr-numbers: add clarity to error messages

per @Insti suggestion.
ocr-numbers: improved wording of error messages
@Insti
Copy link
Contributor Author

Insti commented Sep 24, 2018

Thanks for all your work on this @rpottsoh ❤️

@rpottsoh
Copy link
Member

My pleasure, sorry again about #1337.

@rpottsoh
Copy link
Member

Should trinary and binary be stricken from the list above since they are deprecated?

@ErikSchierboom
Copy link
Member

Should trinary and binary be stricken from the list above since they are deprecated?

I would be fine with that.

Thanks for doing these @rpottsoh!

@rpottsoh
Copy link
Member

If there are no more exercises to be added I think this issue can be closed.

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

4 participants