Skip to main content
repeat eleven
Source Link
Caridorc
  • 28.1k
  • 7
  • 55
  • 138

Unnecessary check / bug

mod can never be \$10\$ in isbn13 (it will always be smaller) because it is defined as x `mod` 10 and: $$ x\ `mod` \ n< \ n$$

So you can just write:

return 10 - mod == int(isbn[-1])

Remove repetition

I like the conditional expression, it is terse and clear, and it can reduce duplication by a lot, but in the below int(val) is repeated twice:

int(val) * 3 if (i + 1) % 2 == 0 else int(val) * 1

This is easy to fix, just make the ternary comprehend just the multiplier:

int(val) * (3 if (i + 1) % 2 == 0 else 1)

or, using a temporary var:

multiplier = 3 if (i + 1) % 2 == 0 else 1
int(val) * multiplier

This makes it extra-clear that int(val) remains the same and only the number it is multiplied with changes.

Base eleven

A special case for X is not needed if you think about it as base eleven, where X is the eleventh digit:

def from_base_eleven(digit):
    return 10 if digit == 'X' else int(digit)

And then:

return mod == from_base_eleven(isbn[-1])

It makes the logic you're following easier to follow.

Unnecessary check / bug

mod can never be \$10\$ in isbn13 (it will always be smaller) because it is defined as x `mod` 10 and: $$ x\ `mod` \ n< \ n$$

So you can just write:

return 10 - mod == int(isbn[-1])

Unnecessary check / bug

mod can never be \$10\$ in isbn13 (it will always be smaller) because it is defined as x `mod` 10 and: $$ x\ `mod` \ n< \ n$$

So you can just write:

return 10 - mod == int(isbn[-1])

Remove repetition

I like the conditional expression, it is terse and clear, and it can reduce duplication by a lot, but in the below int(val) is repeated twice:

int(val) * 3 if (i + 1) % 2 == 0 else int(val) * 1

This is easy to fix, just make the ternary comprehend just the multiplier:

int(val) * (3 if (i + 1) % 2 == 0 else 1)

or, using a temporary var:

multiplier = 3 if (i + 1) % 2 == 0 else 1
int(val) * multiplier

This makes it extra-clear that int(val) remains the same and only the number it is multiplied with changes.

Base eleven

A special case for X is not needed if you think about it as base eleven, where X is the eleventh digit:

def from_base_eleven(digit):
    return 10 if digit == 'X' else int(digit)

And then:

return mod == from_base_eleven(isbn[-1])

It makes the logic you're following easier to follow.

Source Link
Caridorc
  • 28.1k
  • 7
  • 55
  • 138

Unnecessary check / bug

mod can never be \$10\$ in isbn13 (it will always be smaller) because it is defined as x `mod` 10 and: $$ x\ `mod` \ n< \ n$$

So you can just write:

return 10 - mod == int(isbn[-1])