search

exercism / problem-specifications

Shared metadata for exercism exercises.
MIT License
327 stars 541 forks source link

simple-cipher: validating input strings #1298

Closed xarxziux closed 6 years ago

xarxziux commented 6 years ago

I've already raised this as a separate issue in exercism/ecmascript#445, but have just discovered that I should be raising it here.

The unit tests for the Simple Cipher include testing for an error if the cipher is all caps, or all digits, but this is in adequate for the description. Part of the instructions state: "Let's make your substitution cipher a little more fault tolerant by ... ensuring that the key contains only lowercase letters".

Unfortunately this is not checked for, and I've seen one case where the submission passed the tests but did not conform to the description. The addition of tests for spaces or punctuation marks in the key would have failed the submission.

The specification needs to be updated to include these kind of checks.

rpottsoh commented 6 years ago

description testdata

rpottsoh commented 6 years ago

@xarxziux thanks for bringing this issue to light. Would you be willing to submit a PR that fixes this issue?

xarxziux commented 6 years ago

Willing: yes, able: not at the moment. If you're OK with waiting a week or so, then I'll put something up.

rpottsoh commented 6 years ago

There is no rush.

Insti commented 6 years ago

I'm in favour of removing the case checking from the description rather than adding it to the tests. This exercise is about simple cipher not string validation.

See also: https://github.com/exercism/problem-specifications/issues/523 (which might not support my argument.)

Insti commented 6 years ago

See also: https://github.com/exercism/problem-specifications/issues/902

rpottsoh commented 6 years ago

1346 has been merged and #1345 closed, which should now close this issue as well.