hamming: create exercise#42
Conversation
sshine
left a comment
sshine
left a comment
There was a problem hiding this comment.
Nice.
A few clarifying questions, and a superfluous linebreak, but otherwise good to go.
| } | ||
| if {[string length $left] != [string length $right]} { | ||
| error "left and right strands must be of equal length" | ||
| } |
There was a problem hiding this comment.
Looking a few example solutions for other tracks through, does it not suffice to only check for equal length?
With this in mind, the error message will be more generic.
There was a problem hiding this comment.
True. The "check for left/right empty" tests were the most recently added to the canonical data: https://github.com/exercism/problem-specifications/blame/master/exercises/hamming/canonical-data.json
There was a problem hiding this comment.
What's your thinking about keeping/omitting tests specified in the canonical data, given that the commit refers to this issue: exercism/haskell#796, and the new test speciications seem specific to the Haskell track.
There was a problem hiding this comment.
Ah yes, those test cases originate from exercism/haskell#796 and I encouraged that they should be canonical. They were added because of a rather specific situation in ML-like languages where this exact input would fall through all listed cases.
I'd consider them as the simplest cases of the equivalence classes of one being longer than the other, and the other being longer than one, rather than separate equivalence classes. But since they were clearly missing once on Haskell, in spite of having other representatives from that class, maybe that's not true.
I'll let you decide and will approve the PR now.
There was a problem hiding this comment.
What's your thinking about keeping/omitting tests
Sorry, I responded later than I realized that you'd asked.
I think we should keep them:
- Unless they're somehow wrong, having more tests is good.
- Just because their need arose in a language with pattern matching, they're still good representatives of an equivalence class that must be handled.
- Avoiding special cases means we can more easily create test generators.
- We don't need to see them as being anything but an example of mismatching lengths, so I'd rather... ohh...
I realize that the error is different in those two. So if we expect our test suite to be auto-generated, and we don't special-case the generator for this exercise, a correct solution would have to special-case the two.
I realize now that when I approved the PR, I was really only thinking about the Haskell track where distance :: String -> String -> Maybe Int, so no distinction whatsoever. Maybe we should amend this on problem-specifications by making it return the same message in all cases?
There was a problem hiding this comment.
I'll let you follow up on problem-specifications. Here, I'll remove the special cases as the length mismatch test will cover all these cases.
| proc hammingDistance {left right} { | ||
| throw {NOT_IMPLEMENTED} "Implement this procedure." | ||
| } | ||
|
|
There was a problem hiding this comment.
What's the reason to use throw vs. error? Should we consistently use one or the other, or is there a reason why throw is better here when error is used in the example solution?
Also, the last linebreak is unnecessary.
There was a problem hiding this comment.
I think you created the first stub .tcl file using throw, and I've been continuing that usage.
throw is good here because it sets the specific $errorCode:
$ tclsh
% catch {error foo} a b
1
% set b
-code 1 -level 0 -errorstack {INNER {returnImm foo {}}} -errorcode NONE -errorinfo {foo
while executing
"error foo"} -errorline 1
%
% catch {throw NOT_IMPLEMENTED foo} a b
1
% set b
-errorcode NOT_IMPLEMENTED -code 1 -level 0 -errorstack {INNER {returnImm foo {-errorcode NOT_IMPLEMENTED}}} -errorinfo {foo
while executing
"throw NOT_IMPLEMENTED foo"} -errorline 1
% set errorCode
NOT_IMPLEMENTED
Do students typically see the example?
There was a problem hiding this comment.
Do students typically see the example?
I don't understand the question. What do you mean?
There was a problem hiding this comment.
Do students typically see the example?
I don't understand the question. What do you mean?
I have throw in the stub and error in the example. I guess I'm unclear whether you are concerned about confusing the students, or about our own internal consistency.
There was a problem hiding this comment.
I understand the question now.
I don't think many students will see the example before they submit, no.
My concern is that students will use throw because it's already there and not know what to put instead of NOT_IMPLEMENTED. I didn't know myself, but it appears that anything can be placed in it and the current test suite will pass.
Looking at the roadmap in #23, I realize that Hamming comes a while after Error Handling, and that if the Error Handling exercise covers both error and throw, we should be fine here.
2 participants
No description provided.