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

hamming: Remove empty strand errors #1522

Closed
wants to merge 1 commit into from

Conversation

m-dango
Copy link
Member

@m-dango m-dango commented May 25, 2019

Closes #1521

@sshine
Copy link
Contributor

sshine commented May 28, 2019

These test cases were added in #1450 after a discussion in exercism/haskell#796 and affects mainly functional programming languages that use pattern matching and recursion. In summary, they do catch errors that the existing ones didn't, and they do make the existing different-length tests somewhat redundant, but as it was also commented in #1450,

I think the other two cases should remain, even though see seem redundant. They are not redundant to each other.

The other two cases are "disallow first strand longer" and "disallow second strand longer".

Perhaps those two should be removed instead, or these two tests can be made optional once #1518 is resolved. Before that, any track is free to not implement them. I understand that with the Perl 5 track, your test generator fetches test cases directly from canonical data. I don't know if you have plans to handle the optional keyword in #1518 in any way.

@SleeplessByte
Copy link
Member

Since these test cases are not superfluous as per #1450, I will close this PR but feel free to re-open if this is worth another look. I think redundancy is okay if it's in some languages -- don't even think the optional flag would be necessary here :)

@petertseng
Copy link
Member

Perhaps the sensible thing to do is to change the written error.

The error that "left strand must not be empty" wasn't the full picture. If it were taken to be the full picture it would be false because clearly the empty+empty case shows that left stand is allowed to be empty in some cases. So the true description is something like "left strand can only be empty if right strand is also empty" or "left strand must not be be empty if right strand is non-empty", but perhaps that was determined to be too hard to parse the conditionals and associated meaning.

Really these are all cases of "left and right strands must be of equal length". I submit that as my suggestion.

@SleeplessByte
Copy link
Member

Really these are all cases of "left and right strands must be of equal length". I submit that as my suggestion.

Right, and the edge cases are left is empty and right is empty, but that's just because we can treat "empty" differently in many languages. So they three tests together are all testing the same thing, but different cases :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

hamming: Empty strand errors don't make sense
4 participants