Open haus opened 5 years ago
Going to rename this slightly, since it's an update to an existing exercise, not a new exercise proposal.
Unfortunately such a change would currently be blocked by #1560, but once that's cleared up adding some optional "bonus" tests for ISBN-13 validation sounds like a great idea that fulfils the promise implied by the description.md copy.
I'll put it on Hold for now.
@yawpitch thanks for tweaking it. and thanks for pointing out that issue. i hadn't seen it before. reading the issue it sounds like implementing this update in a track would be reasonable while waiting for that to be resolved, so i may look into that.
@haus yeah developing this in a track would work nicely, and once you've got the tests established you can reference that here so we can use it when this repo becomes 'unglued' once more.
I have an issue with this exercise that is kind of similar in nature to this issue but slightly different. I thought I should comment here instead of opening a new issue, but feel free to correct me if I should do otherwise.
I think the bonus is very ambiguous and should be removed.
Generate a valid ISBN-13 from the input ISBN-10 (and maybe verify it again with a derived verifier).
Currently GS1 supports 978 or 979. Which one should we use to extend ISBN-10 into ISBN-13? There is also no write-up provided for ISBN-13 (which is the original intention of this issue)
Generate valid ISBN, maybe even from a givven starting ISBN
I am not sure what this meant. Is it that we have a partial ISBN-10, and we are to complete it? Or given the first 9 digits, we are to compute the last digit?
Either way does not make for good exercise.
Also with a bonus section, it is hard for each track to work around them. This is because some track may not want to do these, whie some track might have their own bonus tasks. We might want to delegate that decision to tracks instead. So it is better to remove these.
Generate a valid ISBN-13 from the input ISBN-10 (and maybe verify it again with a derived verifier).
Currently GS1 supports 978 or 979. Which one should we use to extend ISBN-10 into ISBN-13? There is also no write-up provided for ISBN-13 (which is the original intention of this issue)
This could certainly be better clarified, but since only the 978- prefix can be used to roundtrip from ISBN-10 -> ISBN-13 and ISBN-13 -> ISBN-10 there's no real ambiguity; the bonus would be adding functionality to the ISBN-10 functionality to create a compatible 978- prefixed ISBN-13.
Generate valid ISBN, maybe even from a givven starting ISBN
I am not sure what this meant. Is it that we have a partial ISBN-10, and we are to complete it? Or given the first 9 digits, we are to compute the last digit?
I'd assume that's meant to generate a random, but valid, ISBN-10, or to take some partial prefix and produce a valid ISBN-10 from it. I can see the utility of both, though I'm not sure they're necessary bonus steps.
Also with a bonus section, it is hard for each track to work around them. This is because some track may not want to do these, whie some track might have their own bonus tasks. We might want to delegate that decision to tracks instead. So it is better to remove these.
There's a mechanism in place for adding "optional" tests to the canonical-data.json, which individual tracks can selectively implement, or not, as they see fit. In this case no such tests have been added, so this is merely a suggestion in the copy for the exercise. Personally I'd say there's no reason to have the copy there if the optional tests aren't present, so I'd be fine with removing these, but the decision to implement tests for such "bonus" suggestions that are scattered across many exercises are always delegated to the tracks.
This could certainly be better clarified, but since only the 978- prefix can be used to roundtrip from ISBN-10 -> ISBN-13 and ISBN-13 -> ISBN-10 there's no real ambiguity; the bonus would be adding functionality to the ISBN-10 functionality to create a compatible 978- prefixed ISBN-13.
Ah I see I see, I read the spec closely and yes that's true. the ISBN-10 correspond to the ISBN-13 with 978 only. Thanks for the clarification.
Personally I'd say there's no reason to have the copy there if the optional tests aren't present, so I'd be fine with removing these, but the decision to implement tests for such "bonus" suggestions that are scattered across many exercises are always delegated to the tracks.
I agree with this and view it as a reason to remove the bonus.
In view that questions are now in a tree structure, one way is we can make use of it to house bonus questions? E.g. "isbn-converter" as separate question which is unlocked by "isbn-verifier". The current "bonus" structure may be because Exercism was linear, so with this new tree feature maybe we will want to make use of it. What do you think?
Back to the original intention by @haus. We can either implement ISBN-13 verifier in this problem, or create another problem "isbn-13-verifier" that is unlocked by this problem... My opinion is to put them in the same problem, with the same argument as @haus.
If implementing ISBN13 generation doesn’t depend on implementing ISBN10 generation then it would indeed make sense as a secondary exercise unlocked by the first, if we choose to implement it. That said does ISBN13 verification teach any skills that ISBN10 verification does not? Does ISBN10 -> ISBN13 conversion teach new skills? Or is it just an added “wrinkle” to the original problem? I’d argue for a distinct exercise only if it’s not a wrinkle, not dependent, and adds something new to the mix.
@haus wrote:
The algorithm itself isn't very different from isbn 10, but showing how to support both might be a useful endeavor.
Good point. My observation is that the commentary has so far been discovering ways to do that and what benefits each way has, and I'll continue the commentary in this line of thought.
@Teo-ShaoWei wrote:
Currently GS1 supports 978 or 979.
the ISBN-10 correspond to the ISBN-13 with 978 only
If building an ISBN-10 verifier is as simple as
verifyISBN10 :: String -> Bool
verifyISBN10 s = verifyISBN13 ("978-" ++ s)
then making this an optional test case, rather than a separate exercise, could make sense, too. Again, how tracks might structure splitting exercises into sub-exercises or optional extension to existing exercises is probably a point of contention. Either way it might teach avoiding code duplication.
E.g. "isbn-converter" as separate question which is unlocked by "isbn-verifier"
Reiterating @yawpitch's question:
What does "isbn-converter" teach?
In thinking about what an interface for this exercise might look like:
fromISBN10 :: ISBN10 -> ISBN13
toISBN10 :: ISBN13 -> Maybe ISBN10
the exercise almost seems too trivial, if you discount ISBN-13 verification (and special-casing ISBN-10 verification as part of it). If not, then "isbn-converter" maybe doesn't convey that we're adding ISBN-13 verification in the mix, too.
tl;dr, having explored it a bit I don't think we should implement additional exercises, but neither should we remove the bonus material.
Having now actually bothered to try implementing the bonus, I'm guessing that the reason that this was originally added as a "bonus" specifically rather than as a new exercise was because there's extremely little meaningful difference between the two algorithms.
To validate either you need to be able to:
So, fundamentally, the most significant difference is zipping and multiply the digits with a multiplier mask composed of either a descending range [10,9,8,7,6,5,4,3,2,1]
(ISBN-10) or a repeating cycle [1,3,1,3,1,3,1,3,1,3,1,3,1]
(ISBN-13). Since the multiplier mask implementation can be as straightforward as a literal array they're both very trivial versions of each other ... if you've implemented a valid ISBN-10 verifier you've implemented 99% of an ISBN-13 verifier, so extending your isbn_verifier
solution to handle both iSBN-10 and ISBN-13 is really pretty trivial, and IMHO doesn't reach the bar of deserving a follow up.
ISBN-10 -> ISBN-13 conversion mainly re-capitulates the work done in an ISBN-13 validator ... you strip off the last digit of the ISBN-10, prepend "978" to the front and then work out what the check digit should be using the same [1,3...3,1]
multiplier array used above. It's again a very incremental bit of work, but doesn't seem to add any truly new skills from work done in ISBN-13 validation.
ISBN-13 -> ISBN-10 conversion is slightly more tricky, but only because you need to validate that the first three digits are "978" ... by the time you can do this exercise at all you've got to have the skills to do that, so nothing added there either.
So there are, potentially, four ISBN-centric exercises we could implement, if we wanted to be particularly granular, but I for one cannot see any pedagogic value for the students in us doing so.
Thus I think our real options are:
description.md
.canonical-data.json
to support it.IMHO the latter would be preferable; if you were sitting down to write an ISBN library it would very likely incorporate verification of both ISBN-10 and ISBN-13 as well as ISBN-10 <--> ISBN-13 round-tripping -- it probably would not handle ISBN generation, since that requires access to a proprietary database -- so I think there's pedagogic value in giving the tracks the option of implementing additional tests for which canonical data exists, because they can use that to teach concepts like module creation, namespace management, etc., but I do not personally believe that either implementing new exercises or removing the bonus entirely are warranted.
Finally, #1560 definitely blocks any of these changes, so I think we can end discussion here and return to it at a later date.
The isbn-verifier exercise currently lists converting an isbn10 number to isbn 13 a bonus task. I wonder if adding isbn13 support to the verifier would be too much or would be valuable to the exercise. The algorithm itself isn't very different from isbn 10, but showing how to support both might be a useful endeavor.