hamming: align with specification per #504 #556
Conversation
IMO, empty means |
|
|
||
| static void test_large_hamming_distance(void) | ||
| static void test_disallow_left_empty_strand(void) | ||
| { | ||
| TEST_IGNORE(); | ||
| TEST_ASSERT_EQUAL(4, compute("GATACA", "GCATAA")); | ||
| TEST_ASSERT_EQUAL(-1, compute(NULL, "G")); | ||
| } | ||
|
|
||
| static void test_hamming_distance_in_very_long_strand(void) | ||
| static void test_disallow_right_empty_strand(void) | ||
| { | ||
| TEST_IGNORE(); | ||
| TEST_ASSERT_EQUAL(9, compute("GGACGGATTCTG", "AGGACGGATTCT")); | ||
| TEST_ASSERT_EQUAL(-1, compute("G", NULL)); | ||
| } |
There was a problem hiding this comment.
Well, now I'm having second thoughts about my earlier comment regarding "" and NULL. I don't think that the spec makes much sense here. The name and the check don't match. An empty strand ("") shouldn't be an error , but a missing strand (NULL) should be. Further confusing things is that because the strand lengths don't match, it would be an error regardless of whether it was empty or missing so it's confusing to say that an empty strand should be treated as an error.
I think these tests would make far more sense if named test_disallow_<which>_strand_missing.
There was a problem hiding this comment.
Agreed, the logic means that these two tests don't make sense.
I've removed them and set the related UUIDs to false in the TOML file.
Have also opened an issue on the problem specs repo about it (exercism/problem-specifications#1761).
Fixes #504
Again here the question of semantics in the spec.
Does empty mean a zero-length string (i.e. ""), or NULL?