99cf752ccd3e3810575307d2882d2fa7

I created this function to convert ISBN10 to ISBN13.

Could it be improved?

Will

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
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 !

Ba4e30ee132c4bd4b3fd65ffc5f551e3

typefreak

September 30, 2007, September 30, 2007 07:45, permalink

No rating. Login to rate!

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.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
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
}
Avatar

Daniel K

September 30, 2007, September 30, 2007 12:51, permalink

No rating. Login to rate!

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. 

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
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);
}
90af68f4da3b83257223b871cef473ec

leighmac

October 1, 2007, October 01, 2007 07:00, permalink

No rating. Login to rate!

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.

Goes in your function lib

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
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;    
  }

could use it like this in your template

1
2
3
4
5
6
7
8
9
10
11
12
$converted_to_isbn13 = isbn10_to_13("0901690546");
 
 if($converted_to_isbn13) {
 
   echo $converted_to_isbn13;
 
 } else {
 
   echo 'Opps please check your digits!';
 
 }
D8941b726b5851b8ebad73f458e58268

Mike Cochrane

October 3, 2007, October 03, 2007 00:27, permalink

No rating. Login to rate!

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. 

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
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;
}
D8941b726b5851b8ebad73f458e58268

Mike Cochrane

October 3, 2007, October 03, 2007 00:50, permalink

2 ratings. Login to rate!

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.

# Unrolled loop

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
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;
}

# Unrolled and optimized

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
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;
}
90af68f4da3b83257223b871cef473ec

leighmac

October 3, 2007, October 03, 2007 01:55, permalink

No rating. Login to rate!

Oh my goodness Mike you optimized my codes :) I like the loop unrolling implementation very much and the pre compute is brilliant great job!

7eccce6ad08a0a51963e68f675b74e2f

James Long

October 3, 2007, October 03, 2007 09:13, permalink

1 rating. Login to rate!

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"

D8941b726b5851b8ebad73f458e58268

Mike Cochrane

October 3, 2007, October 03, 2007 17:25, permalink

No rating. Login to rate!

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.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
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;
}
A3537b3cbab6644b6438ac2e03d22821

Michael

March 13, 2008, March 13, 2008 16:15, permalink

No rating. Login to rate!

Now what if you wanted to opposite of this??? ;)

Your refactoring





Format Copy from initial code

or Cancel