isbn-verifier: Include more succinct description of ISBN-10. #1019

sjwarner-bp · 2017-11-21T11:29:34Z

Fixes #1011 .

Insti · 2017-11-21T11:37:42Z

exercises/isbn-verifier/description.md

@@ -1,4 +1,7 @@
-Check if a given ISBN-10 is valid.
+Given a number, check if a given ISBN-10 is valid.


I'm not sure this line's change is an improvement.

Actually the whole line can probably be removed.

Sure thing, I was just trying to mirror the Luhn description, as I find it a really good example for this kind of problem.

Insti · 2017-11-21T11:40:47Z

exercises/isbn-verifier/description.md

+
+## Example
+
+Let's take a random ISBN-10 number, for instance `3-598-21508-8`.


It's not really a random ISBN-10 number.
This line is unnecessarily verbose.

I agree, that's why I slimmed it down. Do you think it should be reduced even more?

Yes.
"For the ISBN-10 3-598..."

Be careful using "ISBN Number" since it has the "ATM Machine" issue.

RAS Syndrome! Good point - I'll strip that too.

Insti · 2017-11-21T11:41:32Z

exercises/isbn-verifier/description.md

+
+## Example
+
+Let's take a random ISBN-10 number, for instance `3-598-21508-8`.
 The first digit block indicates the group where the ISBN belongs. Groups can consist of shared languages, geographic regions or countries. The leading '3' signals this ISBN is from a german speaking country.
 The following number block is to identify the publisher. Since this is a three digit publisher number there is a 5 digit title number for this book.
 The last digit in the ISBN is the check digit which is used to detect read errors.


All this stuff about countries and publishers doesn't belong in the example any more.

Again, I agree but was not confident in stripping it out without another opinion. If you think this should be removed then I'll do that too! 🙂

The actual details of each section aren't relevant to the problem so removing them completely is not a bad option.

Another option is moving them up into the "about ISBN" section if we want to do more than just refer people to Uncyclopedia.

Insti · 2017-11-21T11:43:39Z

exercises/isbn-verifier/description.md

+Given a number, check if a given ISBN-10 is valid.
+
+The [ISBN-10 verification process](http://www.hahnlibrary.net/libraries/isbncalc.html) requires use of a simple formula, and is mostly used to validate book identification
+numbers.



I'd re-phrase this, to focus on what the purpose of ISBNs are before talking about formulas.

Ok, I'll give the problem context first 👍

Insti · 2017-11-21T11:44:20Z

exercises/isbn-verifier/description.md

+```
+
+If the result is 0, then it is a valid ISBN-10, otherwise it is invalid.
+


This is a good description 👍

Insti · 2017-11-21T11:45:13Z

exercises/isbn-verifier/description.md


-Which proves that the ISBN is valid.
+Since the result is 0, this proves that our ISBN is valid.


Good clarification 👍

Should there be one = or two at the end of the line? Line 18 only has one.

See previous comment re: line 9.

Insti · 2017-11-21T11:50:22Z

exercises/isbn-verifier/description.md

-Check if a given ISBN-10 is valid.
+Given a number, check if a given ISBN-10 is valid.
+
+The [ISBN-10 verification process](http://www.hahnlibrary.net/libraries/isbncalc.html) requires use of a simple formula, and is mostly used to validate book identification


Link to wikipedia and solve all your describe 'ISBN' problems: https://en.wikipedia.org/wiki/International_Standard_Book_Number

coriolinus · 2017-11-21T11:56:50Z

exercises/isbn-verifier/description.md

-Converting from string to number can be tricky in certain languages.
-It's getting even trickier since the check-digit of an ISBN-10 can be 'X'.
+Converting from strings to numbers can be tricky in certain languages.
+Now, it's even trickier since the check digit of an ISBN-10 may be 'X' (representative of '10').


representing reads more cleanly to me than representative of here.

Insti · 2017-11-21T12:00:01Z

exercises/isbn-verifier/description.md

@@ -1,4 +1,5 @@
-Check if a given ISBN-10 is valid.
+The [ISBN-10 verification process](http://www.hahnlibrary.net/libraries/isbncalc.html) is mostly used to validate book identification


I'd remove this link, and replace it with the wikipedia one. This link doesn't give much useful information about the number and the formula given there conflicts with the one specified here. (1-9) are reversed on that page.

Ah, I didn't notice that! On it.

Insti · 2017-11-21T12:03:14Z

exercises/isbn-verifier/description.md

@@ -1,4 +1,5 @@
-Check if a given ISBN-10 is valid.
+The [ISBN-10 verification process](https://en.wikipedia.org/wiki/International_Standard_Book_Number) is mostly used to validate book identification


"mostly"?
What else is it used for?

Insti · 2017-11-21T12:06:38Z

exercises/isbn-verifier/description.md

@@ -9,27 +10,30 @@ The program should allow for ISBN-10 without the separating dashes to be verifie



While we're here: Line 8 is grammatically odd.
Also this is the first indication that there could be dashes in the number.
Maybe add an example number to the first paragraph.

Insti · 2017-11-21T12:07:56Z

exercises/isbn-verifier/description.md

@@ -1,4 +1,5 @@
-Check if a given ISBN-10 is valid.
+The [ISBN-10 verification process](https://en.wikipedia.org/wiki/International_Standard_Book_Number) is used to validate book identification
+numbers.

 ## Functionality


Do any other description.mds have a Functionality section?

I thought this was odd too. I am not too sure what the original author means - can you think of a better title? 🙂

What about: Move the whole functionality section under the example and rename it to 'Task'?

Insti · 2017-11-21T12:10:40Z

exercises/isbn-verifier/description.md


 ## Caveats

-Converting from string to number can be tricky in certain languages.
-It's getting even trickier since the check-digit of an ISBN-10 can be 'X'.


Add an example valid ISBN with X as a check digit (from the canonical-data.json)
No need to show the calculation.

Insti · 2017-11-21T12:15:28Z

exercises/isbn-verifier/description.md

@@ -30,7 +30,7 @@ Since the result is 0, this proves that our ISBN is valid.
 ## Caveats

 Converting from strings to numbers can be tricky in certain languages.
-Now, it's even trickier since the check digit of an ISBN-10 may be 'X' (representing '10').
+Now, it's even trickier since the check digit of an ISBN-10 may be 'X' (representing '10'). For instance `3-598-21507-X` is a valid ISBN-10.


Insti · 2017-11-21T12:19:41Z

exercises/isbn-verifier/description.md

+Given an unknown string the program should check if the provided string is a valid ISBN-10.
+Putting this into place requires some thinking about preprocessing/parsing of the string prior to calculating the check digit for the ISBN.
+
+The program should allow for ISBN-10 both with and without separating dashes to be verified. For example, `3-598-21508-8` and `3598215088`.



The program should allow for ISBN-10 both with and without separating dashes to be verified. ~~For example, 3-598-21508-8 and 3598215088.~~

Insti · 2017-11-21T12:20:28Z

exercises/isbn-verifier/description.md

@@ -1,35 +1,40 @@
-Check if a given ISBN-10 is valid.
+The [ISBN-10 verification process](https://en.wikipedia.org/wiki/International_Standard_Book_Number) is used to validate book identification
+numbers.


Add:
"These normally contain dashes and look like: 3-598-21508-8"
or
"These often contain dashes and can look like: 3-598-21508-8"
or something :)

Insti · 2017-11-21T12:24:55Z

Looks good, I need to go do some other stuff now.
I'll leave it a while and see what others have to suggest.
Thanks for all your work on this.

Insti · 2017-11-21T12:26:02Z

exercises/isbn-verifier/description.md

+
+Since the result is 0, this proves that our ISBN is valid.
+
+## Test


Insti · 2017-11-21T12:26:41Z

exercises/isbn-verifier/description.md

+
+## Test
+
+Given an unknown string the program should check if the provided string is a valid ISBN-10.


Given a string, check if it is a valid ISBN-10

Insti · 2017-11-21T12:27:26Z

Closing github window 😁

rpottsoh

You all went gang busters over night on this, found my inbox quite "full" 😄
I have some reading to catch up on. Quick glance thus far looks good!

rpottsoh · 2017-11-21T13:48:13Z

exercises/isbn-verifier/description.md


-Which proves that the ISBN is valid.
+Since the result is 0, this proves that our ISBN is valid.


Should there be one = or two at the end of the line? Line 18 only has one.

rpottsoh · 2017-11-21T13:48:52Z

exercises/isbn-verifier/description.md


-Which proves that the ISBN is valid.
+Since the result is 0, this proves that our ISBN is valid.


See previous comment re: line 9.

sjwarner-bp · 2017-11-21T13:56:36Z

Hmm, that is a good point. Obviously this wants to be as language agnostic as possible - do you think '==' or '=' is better?

coriolinus · 2017-11-21T13:57:05Z

I'd prefer `==`.

…

On Tue, Nov 21, 2017 at 2:56 PM, Sam Warner ***@***.***> wrote: Hmm, that is a good point. Obviously this wants to be as language agnostic as possible - do you think '==' or '=' is better? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1019 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHdeTjC-HyStPP6yAc2jm81dvLlKBsaaks5s4taVgaJpZM4QlsBo> .

sjwarner-bp · 2017-11-21T14:00:00Z

I'm inclined to agree with @coriolinus

ErikSchierboom · 2017-11-22T07:58:36Z

exercises/isbn-verifier/description.md

-A valid ISBN-10 is calculated with this formula `(x1 * 10 + x2 * 9 + x3 * 8 + x4 * 7 + x5 * 6 + x6 * 5 + x7 * 4 + x8 * 3 + x9 * 2 + x10 * 1) mod 11 == 0`
-So for our example ISBN this means:
-(3 * 10 + 5 * 9 + 9 * 8 + 8 * 7 + 2 * 6 + 1 * 5 + 5 * 4 + 0 * 3 + 8 * 2 + 8 * 1) mod 11 = 0
+The program should allow for ISBN-10 both with and without separating dashes to be verified.


I think I would rephrase this slightly to emphasize that both are correct (the current wording only implies allowing them to be verified). Maybe something like:

The program should be able to verify ISBN-10 both with and without separating dashes.

ErikSchierboom

Great work!

Insti

Still good 👍

rpottsoh · 2017-11-22T13:46:13Z

Thanks for making this a better exercise @sjwarner-bp!

Fixes exercism/problem-specifications#1011. A simple definition of what the system should do before getting in to exactly what the digits of an ISBN mean would be good, in my opinion. exercism/problem-specifications#1019

Update description.md with more succinct description of ISBN-10.

78f4742

Insti reviewed Nov 21, 2017

View reviewed changes

isbn-verifer: add @Insti 's improvements

6838078

coriolinus reviewed Nov 21, 2017

View reviewed changes

Add @coriolinus ' improvements.

d12ee14

Insti reviewed Nov 21, 2017

View reviewed changes

isbn-verifier: change link in description.md

6cecdd7

Insti reviewed Nov 21, 2017

View reviewed changes

isbn-verifier: update description

69c65fc

Insti reviewed Nov 21, 2017

View reviewed changes

isbn-verifier: improve grammar.

7d36578

Insti reviewed Nov 21, 2017

View reviewed changes

isbn-verifier: add example ISBN-10 with 'X' as check digit

e666992

Insti reviewed Nov 21, 2017

View reviewed changes

isbn-verifier: move description's 'functionality' section and rename

e44ee31

Insti reviewed Nov 21, 2017

View reviewed changes

isbn-verifier: change position of example

4744404

Insti reviewed Nov 21, 2017

View reviewed changes

isbn-verifier: change section name and more small fixes

cd2e16d

coriolinus approved these changes Nov 21, 2017

View reviewed changes

rpottsoh changed the title ~~Update description.md with more succinct description of ISBN-10.~~ isbn-verifier: Include more succinct description of ISBN-10. Nov 21, 2017

rpottsoh requested changes Nov 21, 2017

View reviewed changes

isbn-verifier: make formulae more consistent

64b4e75

Insti approved these changes Nov 21, 2017

View reviewed changes

ErikSchierboom requested changes Nov 22, 2017

View reviewed changes

Add @ErikSchierboom 's improvement

d6c506b

ErikSchierboom approved these changes Nov 22, 2017

View reviewed changes

Insti approved these changes Nov 22, 2017

View reviewed changes

rpottsoh approved these changes Nov 22, 2017

View reviewed changes

rpottsoh merged commit 2d97a9b into exercism:master Nov 22, 2017

petertseng mentioned this pull request Dec 17, 2017

isbn-verifier: Include more succinct description of ISBN-10 exercism/rust#417

Merged

		@@ -1,4 +1,7 @@
		Check if a given ISBN-10 is valid.
		Given a number, check if a given ISBN-10 is valid.


		## Example

		Let's take a random ISBN-10 number, for instance `3-598-21508-8`.

		```

		If the result is 0, then it is a valid ISBN-10, otherwise it is invalid.


		Which proves that the ISBN is valid.
		Since the result is 0, this proves that our ISBN is valid.

		@@ -1,4 +1,5 @@
		Check if a given ISBN-10 is valid.
		The [ISBN-10 verification process](http://www.hahnlibrary.net/libraries/isbncalc.html) is mostly used to validate book identification

		@@ -1,4 +1,5 @@
		Check if a given ISBN-10 is valid.
		The [ISBN-10 verification process](https://en.wikipedia.org/wiki/International_Standard_Book_Number) is mostly used to validate book identification

		@@ -9,27 +10,30 @@ The program should allow for ISBN-10 without the separating dashes to be verifie


		Since the result is 0, this proves that our ISBN is valid.

		## Test


		## Test

		Given an unknown string the program should check if the provided string is a valid ISBN-10.

Uh oh!

isbn-verifier: Include more succinct description of ISBN-10. #1019

isbn-verifier: Include more succinct description of ISBN-10. #1019

Uh oh!

Conversation

sjwarner-bp commented Nov 21, 2017

Uh oh!

Insti Nov 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Insti Nov 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Insti Nov 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Insti Nov 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Insti commented Nov 21, 2017

Uh oh!

Choose a reason for hiding this comment

Insti Nov 21, 2017 •

edited

Loading

Insti Nov 21, 2017 •

edited

Loading

Insti Nov 21, 2017 •

edited

Loading

Insti Nov 21, 2017 •

edited

Loading