Skip to main content
.substr to .slice to fix IE bug
Source Link
delete me
  • 567
  • 2
  • 7
  • jQuery is entirely unnecessary and will just make your solution slower
  • Don't initialize to new Class() - this is not Java. Instead, initialize to the initial value, or undefined (var x;).
  • use a function to encapsulate your verification logic, and to separate it from the DOM interface.
  • don't call me.val() more than once; cache the result
  • instead of the slow $.each, use a simpler loop
  • You already know the length of the id string, so you can get rid of the p%2 check and simply operate twice on each loop, once on the current position and once on p + 1, and increment by 2 each time (this also results in half as many iterations, speeding up the loop even further).
  • Cast using +string (resulting in a number) and '' + number (resulting in a string)
  • split the string into the first 12 and the last 1 at the beginning, so you don't have to keep excluding the last character and re-extracting it.
  • 'string'.substrslice(-1) is a more concise way to access the last character in a string
  • check to make sure that not only is the string 13 characters long, but also that every character is a numeral

With all that and a few stylistic preferences, here's my take on your problem:

function isValidSAID(id) {
    var i, c,
        even = '',
        sum = 0,
        check = id.substrslice(-1);

    if (id.length != 13 || id.match(/\D/)) {
        return false;
    }
    id = id.substr(0, id.length - 1);
    for (i = 0; c = id.charAt(i); i += 2) {
        sum += +c;
        even += id.charAt(i + 1);
    }
    even = '' + even * 2;
    for (i = 0; c = even.charAt(i); i++) {
        sum += +c;
    }
    sum = 10 - ('' + sum).charAt(1);
    return ('' + sum).substrslice(-1) == check;
}

if (!isValidSAID(document.getElementById('id').innerHTML)) {
    alert('Your South African ID is not valid.');
}
  • jQuery is entirely unnecessary and will just make your solution slower
  • Don't initialize to new Class() - this is not Java. Instead, initialize to the initial value, or undefined (var x;).
  • use a function to encapsulate your verification logic, and to separate it from the DOM interface.
  • don't call me.val() more than once; cache the result
  • instead of the slow $.each, use a simpler loop
  • You already know the length of the id string, so you can get rid of the p%2 check and simply operate twice on each loop, once on the current position and once on p + 1, and increment by 2 each time (this also results in half as many iterations, speeding up the loop even further).
  • Cast using +string (resulting in a number) and '' + number (resulting in a string)
  • split the string into the first 12 and the last 1 at the beginning, so you don't have to keep excluding the last character and re-extracting it.
  • 'string'.substr(-1) is a more concise way to access the last character in a string
  • check to make sure that not only is the string 13 characters long, but also that every character is a numeral

With all that and a few stylistic preferences, here's my take on your problem:

function isValidSAID(id) {
    var i, c,
        even = '',
        sum = 0,
        check = id.substr(-1);

    if (id.length != 13 || id.match(/\D/)) {
        return false;
    }
    id = id.substr(0, id.length - 1);
    for (i = 0; c = id.charAt(i); i += 2) {
        sum += +c;
        even += id.charAt(i + 1);
    }
    even = '' + even * 2;
    for (i = 0; c = even.charAt(i); i++) {
        sum += +c;
    }
    sum = 10 - ('' + sum).charAt(1);
    return ('' + sum).substr(-1) == check;
}

if (!isValidSAID(document.getElementById('id').innerHTML)) {
    alert('Your South African ID is not valid.');
}
  • jQuery is entirely unnecessary and will just make your solution slower
  • Don't initialize to new Class() - this is not Java. Instead, initialize to the initial value, or undefined (var x;).
  • use a function to encapsulate your verification logic, and to separate it from the DOM interface.
  • don't call me.val() more than once; cache the result
  • instead of the slow $.each, use a simpler loop
  • You already know the length of the id string, so you can get rid of the p%2 check and simply operate twice on each loop, once on the current position and once on p + 1, and increment by 2 each time (this also results in half as many iterations, speeding up the loop even further).
  • Cast using +string (resulting in a number) and '' + number (resulting in a string)
  • split the string into the first 12 and the last 1 at the beginning, so you don't have to keep excluding the last character and re-extracting it.
  • 'string'.slice(-1) is a more concise way to access the last character in a string
  • check to make sure that not only is the string 13 characters long, but also that every character is a numeral

With all that and a few stylistic preferences, here's my take on your problem:

function isValidSAID(id) {
    var i, c,
        even = '',
        sum = 0,
        check = id.slice(-1);

    if (id.length != 13 || id.match(/\D/)) {
        return false;
    }
    id = id.substr(0, id.length - 1);
    for (i = 0; c = id.charAt(i); i += 2) {
        sum += +c;
        even += id.charAt(i + 1);
    }
    even = '' + even * 2;
    for (i = 0; c = even.charAt(i); i++) {
        sum += +c;
    }
    sum = 10 - ('' + sum).charAt(1);
    return ('' + sum).slice(-1) == check;
}

if (!isValidSAID(document.getElementById('id').innerHTML)) {
    alert('Your South African ID is not valid.');
}
deleted 134 characters in body
Source Link
delete me
  • 567
  • 2
  • 7
  • jQuery is entirely unnecessary and will just make your solution slower
  • Don't initialize to new Class() - this is not Java. Instead, initialize to an empty string (''), 0the initial value, or undefined (var x;).
  • use a function to encapsulate your verification logic, and to separate it from the DOM interface.
  • don't call me.val() more than once; cache the result
  • instead of the slow $.each, use a simpler loop
  • You already know the length of the id string, so you can get rid of the p%2 check and simply operate twice on each loop, once on the current position and once on p + 1, and increment by 2 each time (this also results in half as many iterations, speeding up the loop even further).
  • Cast using +string (resulting in a number) and '' + number (resulting in a string)
  • You don't need so many different variables, though arguably having them is not as bad thing as it can make the code more readable.
  • split the string into the first 12 and the last 1 at the beginning, so you don't have to keep excluding the last character and re-extracting it.
  • 'string'.substr(-1) is a more concise way to access the last character in a string
  • check to make sure that not only is the string 13 characters long, but also that every character is a numeral

With all that and a few stylistic preferences, here's my take on your problem:

function isValidSAID(id) {
    var i, c,
        even = '',
        sum = 0,
        check = id.substr(-1);

    if (id.length != 13 || id.match(/\D/)) {
        return false;
    }
    id = id.substr(0, id.length - 1);
    for (i = 0; c = id.charAt(i); i += 2) {
        sum += +c;
        even += id.charAt(i + 1);
    }
    even = '' + even * 2;
    for (i = 0; c = even.charAt(i); i++) {
        sum += +c;
    }
    sum = 10 - ('' + sum).charAt(1);
    return ('' + sum).substr(-1) == check;
}

if (!isValidSAID(document.getElementById('id').innerHTML)) {
    alert('Your South African ID is not valid.');
}
  • jQuery is entirely unnecessary and will just make your solution slower
  • Don't initialize to new Class() - this is not Java. Instead, initialize to an empty string (''), 0, or undefined (var x;).
  • use a function to encapsulate your verification logic, and to separate it from the DOM interface.
  • don't call me.val() more than once; cache the result
  • instead of the slow $.each, use a simpler loop
  • You already know the length of the id string, so you can get rid of the p%2 check and simply operate twice on each loop, once on the current position and once on p + 1, and increment by 2 each time (this also results in half as many iterations, speeding up the loop even further).
  • Cast using +string (resulting in a number) and '' + number (resulting in a string)
  • You don't need so many different variables, though arguably having them is not as bad thing as it can make the code more readable.
  • split the string into the first 12 and the last 1 at the beginning, so you don't have to keep excluding the last character and re-extracting it.
  • 'string'.substr(-1) is a more concise way to access the last character in a string
  • check to make sure that not only is the string 13 characters long, but also that every character is a numeral

With all that and a few stylistic preferences, here's my take on your problem:

function isValidSAID(id) {
    var i, c,
        even = '',
        sum = 0,
        check = id.substr(-1);

    if (id.length != 13 || id.match(/\D/)) {
        return false;
    }
    id = id.substr(0, id.length - 1);
    for (i = 0; c = id.charAt(i); i += 2) {
        sum += +c;
        even += id.charAt(i + 1);
    }
    even = '' + even * 2;
    for (i = 0; c = even.charAt(i); i++) {
        sum += +c;
    }
    sum = 10 - ('' + sum).charAt(1);
    return ('' + sum).substr(-1) == check;
}

if (!isValidSAID(document.getElementById('id').innerHTML)) {
    alert('Your South African ID is not valid.');
}
  • jQuery is entirely unnecessary and will just make your solution slower
  • Don't initialize to new Class() - this is not Java. Instead, initialize to the initial value, or undefined (var x;).
  • use a function to encapsulate your verification logic, and to separate it from the DOM interface.
  • don't call me.val() more than once; cache the result
  • instead of the slow $.each, use a simpler loop
  • You already know the length of the id string, so you can get rid of the p%2 check and simply operate twice on each loop, once on the current position and once on p + 1, and increment by 2 each time (this also results in half as many iterations, speeding up the loop even further).
  • Cast using +string (resulting in a number) and '' + number (resulting in a string)
  • split the string into the first 12 and the last 1 at the beginning, so you don't have to keep excluding the last character and re-extracting it.
  • 'string'.substr(-1) is a more concise way to access the last character in a string
  • check to make sure that not only is the string 13 characters long, but also that every character is a numeral

With all that and a few stylistic preferences, here's my take on your problem:

function isValidSAID(id) {
    var i, c,
        even = '',
        sum = 0,
        check = id.substr(-1);

    if (id.length != 13 || id.match(/\D/)) {
        return false;
    }
    id = id.substr(0, id.length - 1);
    for (i = 0; c = id.charAt(i); i += 2) {
        sum += +c;
        even += id.charAt(i + 1);
    }
    even = '' + even * 2;
    for (i = 0; c = even.charAt(i); i++) {
        sum += +c;
    }
    sum = 10 - ('' + sum).charAt(1);
    return ('' + sum).substr(-1) == check;
}

if (!isValidSAID(document.getElementById('id').innerHTML)) {
    alert('Your South African ID is not valid.');
}
Source Link
delete me
  • 567
  • 2
  • 7

  • jQuery is entirely unnecessary and will just make your solution slower
  • Don't initialize to new Class() - this is not Java. Instead, initialize to an empty string (''), 0, or undefined (var x;).
  • use a function to encapsulate your verification logic, and to separate it from the DOM interface.
  • don't call me.val() more than once; cache the result
  • instead of the slow $.each, use a simpler loop
  • You already know the length of the id string, so you can get rid of the p%2 check and simply operate twice on each loop, once on the current position and once on p + 1, and increment by 2 each time (this also results in half as many iterations, speeding up the loop even further).
  • Cast using +string (resulting in a number) and '' + number (resulting in a string)
  • You don't need so many different variables, though arguably having them is not as bad thing as it can make the code more readable.
  • split the string into the first 12 and the last 1 at the beginning, so you don't have to keep excluding the last character and re-extracting it.
  • 'string'.substr(-1) is a more concise way to access the last character in a string
  • check to make sure that not only is the string 13 characters long, but also that every character is a numeral

With all that and a few stylistic preferences, here's my take on your problem:

function isValidSAID(id) {
    var i, c,
        even = '',
        sum = 0,
        check = id.substr(-1);

    if (id.length != 13 || id.match(/\D/)) {
        return false;
    }
    id = id.substr(0, id.length - 1);
    for (i = 0; c = id.charAt(i); i += 2) {
        sum += +c;
        even += id.charAt(i + 1);
    }
    even = '' + even * 2;
    for (i = 0; c = even.charAt(i); i++) {
        sum += +c;
    }
    sum = 10 - ('' + sum).charAt(1);
    return ('' + sum).substr(-1) == check;
}

if (!isValidSAID(document.getElementById('id').innerHTML)) {
    alert('Your South African ID is not valid.');
}