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

Clarify Series errors in README #2469

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Clarify Series errors in README #2469

wants to merge 1 commit into from

Conversation

iHiD
Copy link
Member

@iHiD iHiD commented Aug 13, 2024

Not set on the wording, or which cases we should cover, but putting this here so it can be riffed on by maintainers.

Forum discussion: http://forum.exercism.org/t/series-problem-description-unclear/12477

@iHiD iHiD requested a review from a team as a code owner August 13, 2024 10:56
@glennj
Copy link
Contributor

glennj commented Aug 13, 2024

Perhaps

The series length `n` must not be longer than the string of digits.
Trying to extract a 6-digit series from a 5-digit string should result in an error.

@@ -13,7 +13,7 @@ And the following 4-digit series:
- "4914"
- "9142"

And if you ask for a 6-digit series from a 5-digit string, you deserve whatever you get.
Trying to extract a 6-digit series from a 5-digit string should result in an error.
Copy link
Member

Choose a reason for hiding this comment

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

Is "should result in an error" something that should be track specific behavior?

@tasxatzial
Copy link
Member

Old PR. Should we move it forward?

Simply by looking at the canonical data, it appears that there are even more cases where an error is expected:

  • slice length cannot be zero
  • slice length cannot be negative
  • empty series is invalid

So, it's best to either mention all cases explicitly or remove any reference to errors from the description altogether, which seems more reasonable to me.

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.

5 participants