Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion config.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,21 @@
"transforming"
]
},
{
"uuid": "7961c852-c87a-44b0-b152-efea3ac8555c",
"slug": "isbn-verifier",
"core": false,
"unlocked_by": null,
"difficulty": 1,
"topics": [
"type_conversion",
"conditionals",
"strings",
"arrays",
"integers",
"parsing"
]
},
{
"uuid": "8648fa0c-d85f-471b-a3ae-0f8c05222c89",
"slug": "hamming",
Expand Down Expand Up @@ -1112,7 +1127,12 @@
"unlocked_by": null,
"difficulty": 3,
"topics": [
"exception_handling"
"type_conversion",
"exception_handling",
"strings",
"arrays",
"integers",
"parsing"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you've accidentally included some changes for a different exercise.

]
},
{
Expand Down
43 changes: 43 additions & 0 deletions exercises/isbn-verifier/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# Isbn Verifier

Check if a given ISBN-10 is valid.

## Functionality

Given an unkown 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 without the separating dashes to be verified as well.

## ISBN

Let's take a random ISBN-10 number, say `3-598-21508-8` for this.
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.

The first 9 digits in the ISBN have to be between 0 and 9.
The check digit can additionally be an 'X' to allow 10 to be a valid check digit as well.

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

Which proves that the ISBN is valid.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add something here (and to .meta/hints.md) to explain that we are also expecting implementations of an ISBN10 generator and an ISBN13 generator? It will also need to explain how the conversion from ISBN10 to ISBN13 works, and how to do the ISBN13 checksum calculation.

### Submitting Exercises
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be at heading level 2 (## Submitting Exercises).


Note that, when trying to submit an exercise, make sure the solution is in the `exercism/python/<exerciseName>` directory.

For example, if you're submitting `bob.py` for the Bob exercise, the submit command would be something like `exercism submit <path_to_exercism_dir>/python/bob/bob.py`.


For more detailed information about running tests, code style and linting,
please see the [help page](http://exercism.io/languages/python).

## Source

Converting a string into a number and some basic processing utilizing a relatable real world example. [https://en.wikipedia.org/wiki/International_Standard_Book_Number#ISBN-10_check_digit_calculation](https://en.wikipedia.org/wiki/International_Standard_Book_Number)

## Submitting Incomplete Solutions
It's possible to submit an incomplete solution so you can see how others have completed the exercise.
48 changes: 48 additions & 0 deletions exercises/isbn-verifier/example.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
def verify(isbn):
clear_isbn = _remove_non_alphanumeric(isbn)
if len(clear_isbn) != 10:
return False

isbn_main_part = clear_isbn[:-1]
if not isbn_main_part.isdigit():
return False

calculated_digit = _calculate_isbn10_check_digit(isbn_main_part)
check_digit = clear_isbn[-1:].upper()

return calculated_digit == check_digit


def isbn_generator(isbn):
clear_isbn = _remove_non_alphanumeric(isbn)
if len(clear_isbn) < 9 or len(clear_isbn) > 10:
raise ValueError()

isbn_main_part = clear_isbn[:9]
isbn = isbn_main_part + _calculate_isbn10_check_digit(isbn_main_part)
return '-'.join([isbn[:1], isbn[1:4], isbn[4:9], isbn[9:]])


def isbn13_generator_from_isbn10(isbn10):
clear_isbn = _remove_non_alphanumeric(isbn10)
if len(clear_isbn) != 10:
raise ValueError()

isbn_main_part = '978' + clear_isbn[:9]
isbn = isbn_main_part + _calculate_isbn13_check_digit(isbn_main_part)
return '-'.join([isbn[:3], isbn[3:4], isbn[4:6], isbn[6:12], isbn[12:13]])


def _remove_non_alphanumeric(value):
return ''.join([x for x in str(value) if x.isalnum()])


def _calculate_isbn10_check_digit(isbn):
isbn_sum = sum([int(x) * (i + 1) for i, x in enumerate(isbn)]) % 11
return 'X' if isbn_sum == 10 else str(isbn_sum)


def _calculate_isbn13_check_digit(isbn):
isbn_sum = sum([int(x) * (3 if i % 2 == 1 else 1)
for i, x in enumerate(isbn)]) % 10
return str(isbn_sum) if isbn_sum == 0 else str(10 - isbn_sum)
10 changes: 10 additions & 0 deletions exercises/isbn-verifier/isbn_verifier.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
def verify(isbn):
pass


def isbn_generator(isbn):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generate_isbn might be a better function name? I think function names should be more descriptive of what they do than what they are.

pass


def isbn13_generator_from_isbn10(isbn10):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generate_isbn13_from_isbn10 might be a better function name?

pass
90 changes: 90 additions & 0 deletions exercises/isbn-verifier/isbn_verifier_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import unittest

from isbn_verifier import verify, isbn_generator, isbn13_generator_from_isbn10


# Tests adapted from `problem-specifications//canonical-data.json` @ v1.0.0

class IsbnVerifierTests(unittest.TestCase):

def test_valid_isbn(self):
self.assertIs(verify('3-598-21508-8'), True)

def test_invalid_check_digit(self):
self.assertIs(verify('3-598-21508-9'), False)

def test_valid_with_X_check_digit(self):
self.assertIs(verify('3-598-21507-X'), True)

def test_invalid_check_digit_other_than_X(self):
self.assertIs(verify('3-598-21507-A'), False)

def test_invalid_character_in_isbn(self):
self.assertIs(verify('3-598-2K507-0'), False)

def test_invalid_X_other_than_check_digit(self):
self.assertIs(verify('3-598-2X507-0'), False)

def test_valid_isbn_without_separating_dashes(self):
self.assertIs(verify('3598215088'), True)

def test_valid_isbn_without_separating_dashes_with_X_check_digit(self):
self.assertIs(verify('359821507X'), True)

def test_invalid_isbn_without_check_digit_and_dashes(self):
self.assertIs(verify('359821507'), False)

def test_invalid_too_long_isbn_with_no_dashes(self):
self.assertIs(verify('3598215078X'), False)

def test_invalid_isbn_without_check_digit(self):
self.assertIs(verify('3-598-21507'), False)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you can use AssertTrue and AssertFalse instead of AssertIs.

I.e.:

    def test_verify_isbn_example_with_X_spaces(self):
        self.assertTrue(isbn_verifier.verify_isbn('955 20 3051 X'))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertFalse won't fail the test if the tested method returns None in comparison with assertIs

Consider this:

import unittest

tc = unittest.TestCase()

tc.assertFalse(None) # passes
tc.assertIs(None, True) # fails as expected

Do you see the difference, @jamesmcm?

So, I think it's better to use assertIs even if readability suffers a bit

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point, seems like strange behaviour from the unittest module.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, especially considering that bool(None) returns False meaning that None is Falsy

Copy link
Copy Markdown
Contributor

@N-Parsons N-Parsons Oct 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@m-a-ge, your example code is asserting that None is Falsey, so passing is the expected behaviour - bool(None) is False, so assertFalse(None) should pass (and does).

import unittest

tc = unittest.TestCase()

tc.assertFalse(None)  # Passes, since None is Falsey
tc.assertTrue(None)   # Fails because None is not Truthy

tc.assertIs(None, False)  # Fails because None is not False
tc.assertIs(None, True)   # Fails because None is not True

In theory, there's nothing wrong with using assertTrue and assertFalse in exercise tests, but the presence of the latter will cause some tests to pass when no code is implemented, and so these could confuse some people. Using assertIs creates another slight issue, in that it forces learners to return booleans, even though they may expect to be able to use Truthy and Falsey statements instead. I'd consider assertIs the lesser of two evils.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@N-Parsons thanks for clarification


def test_invalid_too_long_isbn(self):
self.assertIs(verify('3-598-21507-XA'), False)

def test_invalid_check_digit_X_used_for_0(self):
self.assertIs(verify('3-598-21515-X'), False)


class IsbnGeneratorTests(unittest.TestCase):

def test_valid_isbn(self):
self.assertEqual(isbn_generator('3-598-21508-8'), '3-598-21508-8')

def test_valid_isbn_without_separating_dashes_with_X_check_digit(self):
self.assertEqual(isbn_generator('359821507X'), '3-598-21507-X')

def test_valid_isbn_without_check_digit(self):
self.assertEqual(isbn_generator('359821508'), '3-598-21508-8')

def test_invalid_isbn_too_short(self):
with self.assertRaises(Exception):
isbn_generator('3-598-2350')

def test_invalid_isbn_too_long(self):
with self.assertRaises(Exception):
isbn_generator('359723508XA')


class Isbn13GeneratorTests(unittest.TestCase):

def test_valid_isbn(self):
self.assertEqual(isbn13_generator_from_isbn10(
'3-598-21508-8'), '978-3-59-821508-7')

def test_valid_isbn_without_separating_dashes_with_X_check_digit(self):
self.assertEqual(isbn13_generator_from_isbn10(
'359821507X'), '978-3-59-821507-0')

def test_invalid_isbn_too_short(self):
with self.assertRaises(Exception):
isbn13_generator_from_isbn10('3-598-23506')

def test_invalid_isbn_too_long(self):
with self.assertRaises(Exception):
isbn13_generator_from_isbn10('3597235089X')


if __name__ == '__main__':
unittest.main()