function isbn10_to_13($isbnold){
if (strlen($isbnold) != 10){ // Make sure we have a 10 digit string to start
return 'Invalid ISBN-10, must be 10 digits';
}
// prefix with 978 and drop old checksum (last digit)
$isbn = '978'.substr($isbnold,0,9);
for ($i = 0; $i <= 12; $i++){ // For each digit if new isbn
$weight = ($i%2 == 0)? 1 : 3; // Alternate between 1's and 3's
$check_sum_total = $check_sum_total + ($isbn{$i} * $weight); // multiply each digit by 1 or 3 and add to $checksumtotal
}
$new_check_sum = 10 - ($check_sum_total%10); // Modulus 10 business
return ($isbn.$new_check_sum); //add checksum on to end and return
}
Refactorings
No refactoring yet !
typefreak
September 30, 2007, September 30, 2007 07:45, permalink
1: You don't check if all characters are digits (only the length of the string)
2: Use [] to acces a string character, instead of {} (preferred, not mandatory)
3: Initialise variables (good practice, neccesary if using error_reporting on E_ALL)
From the php-manual:
Note: They may also be accessed using braces like $str{42} for the same purpose. However, using square array-brackets is preferred because the {braces} style is deprecated as of PHP 6.
function isbn10_to_13($isbnold){
if (!preg_match('#^[0-9]{10}$#', $isbnold)) {
// Input string is of a wrong format
return 'Invalid ISBN-10, must be 10 digits';
}
// Don't forget to set checksumtotal to initial value
$check_sum_total = 0;
// prefix with 978 and drop old checksum (last digit)
$isbn = '978'.substr($isbnold,0,9);
for ($i = 0; $i <= 12; $i++){ // For each digit of new isbn
// multiply each digit by 1 or 3 and add to $checksumtotal
$check_sum_total = $check_sum_total + ($isbn[$i] * (($i % 2 == 0) ? 1 : 3) );
}
$new_check_sum = 10 - ($check_sum_total%10); // Modulus 10 business
return ($isbn.$new_check_sum); //add checksum on to end and return
}
Daniel K
September 30, 2007, September 30, 2007 12:51, permalink
There are some errors in your code. The new ISBN code will be 12 digits long without the checksum, but you loop over 13 digits (0..12). 2) If $check_sum_total % 10 == 0, $new_check_sum will be 10 rather than 0. I fixed these in the first version.
For fun, I also added a version in which the checksum of the '978' doesn't need to be calculated.
function isbn10_to_13($isbnold){
if (!preg_match('#^[0-9]{10}$#', $isbnold)) {
// Input string is of a wrong format
return 'Invalid ISBN-10, must be 10 digits';
}
// Don't forget to set checksumtotal to initial value
$check_sum_total = 0;
// prefix with 978 and drop old checksum (last digit)
$isbn = '978'.substr($isbnold,0,9);
for ($i = 0; $i <= 11; $i++){ // For each digit of new isbn
// multiply each digit by 1 or 3 and add to $checksumtotal
$check_sum_total = $check_sum_total + ($isbn[$i] * (($i % 2 == 0) ? 1 : 3) );
}
$new_check_sum = 9 - (($check_sum_total + 9) %10); // Modulus 10 business
return ($isbn.$new_check_sum); //add checksum on to end and return
}
function isbn10_to_13($isbnold){
if (!preg_match('#^[0-9]{10}$#', $isbnold)) {
// Input string is of a wrong format
return 'Invalid ISBN-10, must be 10 digits';
}
// Don't forget to set checksumtotal to initial value
$check_sum_total = 0;
for ($i = 0; $i <= 8; $i++){ // For each digit of new isbn
// multiply each digit by 1 or 3 and add to $checksumtotal
$check_sum_total = $check_sum_total + ($isbn[$i] * (($i % 2 == 0) ? 3 : 1) );
}
//Add 8 to check sum total for initial '978', subtract 1 from both sides to keep under 10.
$new_check_sum = 9 - (($check_sum_total + 7) % 10);
return ('978'. substr($isbnold,0,9) . $new_check_sum);
}
leighmac
October 1, 2007, October 01, 2007 07:00, permalink
I would write that function like this. I just changed a few things to make it easier to read and use for me and removed the error message out of the function making it more flexible IMO.
function isbn10_to_13($isbn)
{
if (!preg_match('{^[0-9]{10}$}', $isbn))
{
# number is not 10 digits
return false;
}
# prefix 978 (bookland flag) to isbn and drop last digit (check digit)
$isbn = '978'.substr($isbn,0,9);
$sum_of_digits = 0;
$digit_postion = 0;
# loop over each isbn digit
foreach (str_split($isbn) as $digit)
{
# multiply each digit by its associated weighting factor (1 or 3)
# and add them all together
$sum_of_digits += $digit * (($digit_postion % 2 == 0) ? 1 : 3);
$digit_postion ++;
}
# divide the sum_of_digits by the modulus number (10) to find the remainder
# and then minus 10 to get the check digit
$check_digit = 10 - ($sum_of_digits % 10);
# return isbn with check_digit
return $isbn.$check_digit;
}
$converted_to_isbn13 = isbn10_to_13("0901690546");
if($converted_to_isbn13) {
echo $converted_to_isbn13;
} else {
echo 'Opps please check your digits!';
}
Mike Cochrane
October 3, 2007, October 03, 2007 00:27, permalink
If speed is important (eg converting many thousands of these), then here is my version. I started with leighmac's code and then optimized from there. Changes:
Removed expensive str_split and replaced with a for loop (we already know all the characters will be there) (13% time saving).
Move ternary '($digit_postion % 2 == 0)' check so we don't do and extra '* 1' operation (4.5% time saving)
Remove expensive substr and have preg_match split the first 9 chars off (22% time saving)
Use a bitwise '& 1' comparison instead of '% 2' (3.1% time saving)
Total saving 38% on the leighmac's code.
function isbn10_to_13($isbn) {
if (!preg_match('{^([0-9]{9})[0-9]$}', $isbn, $matches)) {
# number is not 10 digits
return false;
}
# prefix 978 (bookland flag) to isbn and drop last digit (check digit)
$isbn = '978' . $matches[1];
# loop over each isbn digit
$sum_of_digits = 0;
for ($digit_postion = 0; $digit_postion < 12; $digit_postion++) {
# multiply each digit by its associated weighting factor (1 or 3)
# and add them all together
$sum_of_digits += ($digit_postion & 1) ? 3 * $isbn{$digit_postion} : $isbn{$digit_postion};
}
# divide the sum_of_digits by the modulus number (10) to find the remainder
# and then minus 10 to get the check digit
$check_digit = 10 - ($sum_of_digits % 10);
# return isbn with check_digit
return $isbn . $check_digit;
}
Mike Cochrane
October 3, 2007, October 03, 2007 00:50, permalink
So if you use a technique often used in optimizing cryptographic code call 'Loop unrolling' and them simplify you get the first block below. This is a further saving of 17% on my fastest code above.
But then you'll see that we're always doing the same things to the 978 digits. Why not pre compute this. Then then we get (with a few other tweaks like taking the matches out of the preg_match etc) the second code below.
And this if a further 8% time saving.
So overall we've save a huge 52% on leightmac's code.
Now there wasn't anything wrong with the code that I started with. It was great and perfect for those one off situations. I did this more in as an exercise to have people think about what can be done if you have to.
function isbn10_to_13($isbn) {
if (!preg_match('{^([0-9]{9})[0-9]$}', $isbn, $matches)) {
# number is not 10 digits
return false;
}
# prefix 978 (bookland flag) to isbn and drop last digit (check digit)
$isbn = '978' . $matches[1];
# sum the digits with their weights
$sum_of_digits = 3 * ($isbn{1} + $isbn{3} + $isbn{5} + $isbn{7} + $isbn{9} + $isbn{11}) +
$isbn{0} + $isbn{2} + $isbn{4} + $isbn{6} + $isbn{8} + $isbn{10};
# divide the sum_of_digits by the modulus number (10) to find the remainder
# and then minus 10 to get the check digit
$check_digit = 10 - ($sum_of_digits % 10);
# return isbn with check_digit
return $isbn . $check_digit;
}
function isbn10_to_13($isbn) {
if (!preg_match('{^[0-9]{10}$}', $isbn)) {
# number is not 10 digits
return false;
}
# sum the digits with their weights and add the checksum for the 978 prefix
$sum_of_digits = 38 + 3 * ($isbn{0} + $isbn{2} + $isbn{4} + $isbn{6} + $isbn{8}) +
$isbn{1} + $isbn{3} + $isbn{5} + $isbn{7};
# divide the sum_of_digits by the modulus number (10) to find the remainder
# and then minus 10 to get the check digit
$check_digit = 10 - ($sum_of_digits % 10);
# return isbn with check_digit
return '978' . $isbn . $check_digit;
}
leighmac
October 3, 2007, October 03, 2007 01:55, permalink
Oh my goodness Mike you optimized my codes :) I like the loop unrolling implementation very much and the pre compute is brilliant great job!
James Long
October 3, 2007, October 03, 2007 09:13, permalink
The last "digit" of the ISBN-10 (the checksum) ranges from 0-10, with 10 represented by X. For ISBN-13, it ranges from 1-10 with 10 represented by A. The latter was never taken into account in the original - the former was broken in most of the refactorings by people trying to enforce "digits"
Mike Cochrane
October 3, 2007, October 03, 2007 17:25, permalink
Thanks James. You're right on the ISBN-10 check sum range. For ISBN-13 is ranges from 0-9. There is no A according to http://www.isbn-international.org/ or Wikipedia. Thanks for pointing this out.
So my previous solution was incorrect. Here is a correct solution. Performance hit in the fix though.
So final result is 45% time saving on leightmac's but it doesn't reject legit numbers and the check digit is always a single digit, not '10' like it could have previously been.
function isbn10_to_13($isbn) {
if (!preg_match('{^([0-9]{9})[0-9xX]$}', $isbn, $matches)) {
# number is not 10 digits
return false;
}
# sum the digits with their weights and add the checksum for the 978 prefix
$sum_of_digits = 38 + 3 * ($isbn{0} + $isbn{2} + $isbn{4} + $isbn{6} + $isbn{8}) +
$isbn{1} + $isbn{3} + $isbn{5} + $isbn{7};
# divide the sum_of_digits by the modulus number (10) to find the remainder
# and then minus 10 to get the check digit
$check_digit = (10 - ($sum_of_digits % 10)) % 10;
# return isbn with check_digit
return '978' . $matches[1] . $check_digit;
}
Michael
March 13, 2008, March 13, 2008 16:15, permalink
Now what if you wanted to opposite of this??? ;)
asimshahiddit.myopenid.com
April 21, 2011, April 21, 2011 00:24, permalink
convert isbn13 to isbn10
function isbn13_to_10($_isbn13){
if (strlen( $_isbn13 ) != 13 )return $_isbn13;
$_isbn10=substr($_isbn13,3,9);
$checkSum=0;
$weight = 10;
for($i=0;$i<strlen($_isbn10);$i++){
$checkSum+=intval($_isbn10[$i]) * $weight;
$weight--;
}
$checkSum = 11 - ( $checkSum % 11 ) ;
if ( $checkSum == 10 ) {$_isbn10=$_isbn10 . "X"; }
else if ($checkSum == 11 ) {
$_isbn10 = $_isbn10 . "0";
}else {
$_isbn10 = $_isbn10 . $checkSum;
}
return $_isbn10;
}
asimshahiddit.myopenid.com
April 21, 2011, April 21, 2011 00:24, permalink
convert isbn13 to isbn10
function isbn13_to_10($_isbn13){
if (strlen( $_isbn13 ) != 13 )return $_isbn13;
$_isbn10=substr($_isbn13,3,9);
$checkSum=0;
$weight = 10;
for($i=0;$i<strlen($_isbn10);$i++){
$checkSum+=intval($_isbn10[$i]) * $weight;
$weight--;
}
$checkSum = 11 - ( $checkSum % 11 ) ;
if ( $checkSum == 10 ) {$_isbn10=$_isbn10 . "X"; }
else if ($checkSum == 11 ) {
$_isbn10 = $_isbn10 . "0";
}else {
$_isbn10 = $_isbn10 . $checkSum;
}
return $_isbn10;
}
asimshahiddit.myopenid.com
April 21, 2011, April 21, 2011 00:24, permalink
convert isbn13 to isbn10
function isbn13_to_10($_isbn13){
if (strlen( $_isbn13 ) != 13 )return $_isbn13;
$_isbn10=substr($_isbn13,3,9);
$checkSum=0;
$weight = 10;
for($i=0;$i<strlen($_isbn10);$i++){
$checkSum+=intval($_isbn10[$i]) * $weight;
$weight--;
}
$checkSum = 11 - ( $checkSum % 11 ) ;
if ( $checkSum == 10 ) {$_isbn10=$_isbn10 . "X"; }
else if ($checkSum == 11 ) {
$_isbn10 = $_isbn10 . "0";
}else {
$_isbn10 = $_isbn10 . $checkSum;
}
return $_isbn10;
}
I created this function to convert ISBN10 to ISBN13.
Could it be improved?
Will