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

simple-cipher: remove checks for invalid input #1346

Merged
merged 2 commits into from
Oct 22, 2018

Conversation

rpottsoh
Copy link
Member

@rpottsoh rpottsoh commented Oct 2, 2018

  • Mention of invalid input removed from README.
  • Removed cases that test for invalid input.

These changes follow on the discussion that has taken place in #902 and #523. There may be some other examples regarding the discussion that I am missing. Feel free to point them out.

This PR is intended to be an alternate solution to #1298 to replace #1316.

@rpottsoh rpottsoh requested review from Insti and petertseng October 9, 2018 16:25
@petertseng petertseng removed their request for review October 9, 2018 16:30
@Insti
Copy link
Contributor

Insti commented Oct 9, 2018

I like this change.

Perhaps we (@xarxziux?) can make a new exercise called 'key-validation' or something that combines these tests with the tests that they added in #1346 since string validation is still an important thing to learn.

@xarxziux
Copy link

xarxziux commented Oct 9, 2018

Do you mean an advanced version of this exercise (not-so-simple-cipher?), or an entirely separate exercise that only performs general data validation?

I think having a tiered version of these kind of exercises would be good in principle - whether that is in the form of a separate, more advanced exercise, or optional test cases. I'm just not sure that the format of this site really supports either. Is there an accepted way to declare optional or advanced test cases in the spec?

@rpottsoh
Copy link
Member Author

A new exercise could be a side exercise and it would either require that the student have already completed this exercise in order to have the code necessary for ciphering or the new exercise could provide a stub that contained code for ciphering. The student would be tasked with writing the code to protect the ciphering logic from bad input. If the ciphering logic is provided then the new exercise would be independent of this exercise... I am not sure which way is the best to go.

@rpottsoh rpottsoh force-pushed the removeSimpleCipherErrorChecking branch from 42dd863 to be80634 Compare October 10, 2018 19:09
@xarxziux
Copy link

On another coding site, Codewars, there are a number of exercises that form a series. You first take on a relatively easy problem. Once you've solved that, then there's a more difficult version so solve and so on. This site doesn't really have that concept, but maybe it would not be a bad idea to do something like that.

The description on this exercise already has three stages. If we are making the requirements easier, instead of making the unit tests harder, maybe we should look at serialising the exercise. Instead of steps 1 to 3, split it into two or three separate exercises with increasing difficulty. I'm not sure there's enough material in the current exercise to warrant three exercises, but I'm sure there are extra complexities and restrictions that can be made on the final stage to make it work.

@rpottsoh
Copy link
Member Author

rpottsoh commented Oct 10, 2018 via email

@Insti
Copy link
Contributor

Insti commented Oct 10, 2018

I meant an entirely new and separate exercise that just performs data validation.

It should work standalone, but would be ok if it is part of a series.

@rpottsoh rpottsoh force-pushed the removeSimpleCipherErrorChecking branch from be80634 to 2016d79 Compare October 15, 2018 14:18
@rpottsoh rpottsoh force-pushed the removeSimpleCipherErrorChecking branch from 2016d79 to 5eb79cf Compare October 16, 2018 15:01
@rpottsoh rpottsoh self-assigned this Oct 16, 2018
@rpottsoh rpottsoh merged commit f7f3659 into exercism:master Oct 22, 2018
@rpottsoh rpottsoh deleted the removeSimpleCipherErrorChecking branch October 22, 2018 18:58
topstack1226 added a commit to topstack1226/typescript that referenced this pull request Jan 10, 2023
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