Home / Questions / 22376
Accepted
Juan Pinzón
Asked: 2020-09-02 21:21:38 +0800 CST 2020-09-02 21:21:38 +0800 CST

Optimize algorithm implementation

  • 772

The following algorithm checks if a string is valid following the following rules:

Given a string of 10 numeric characters:

  • The tenth numeric character is the result of a calculation and the check digit
  • It works with the first 9 numeric characters of the string.
  • Each digit of even position (taking into account that the first numeric character is in position 0) is multiplied by two, if this result is greater than nine, nine is subtracted.
  • Each digit of odd position (taking into account that the first numeric character is in position 0) is multiplied by one.
  • All the results of the multiplications carried out previously are added.
  • The result of the previous sum is subtracted from the next higher ten.
  • The above result must be the tenth digit.
  • If the final subtraction is 10, the tenth digit is zero.

EXAMPLE

0   7   0   4   1   6   3   5   7   5
*   *   *   *   *   *   *   *   *   
2   1   2   1   2   1   2   1   2      se multiplica caracteres pares * 2 y si es > 9 se resta nueve, e impares * 1 
_____________________________________
0   7   0   4   2   6   6   5   5   
|_______________________________|      se suma el resultado de las multiplicaciones, en este caso es = 35
               35                      Este resultado se resta de la decena inmediata superior, es decir 40 - 35 = 5, 
                                       y este resultado debe ser = al ultimo caracter de la cadena, en este caso 5. 
                                       Por lo que 5=5, comprobando que la cadena es válida.

Examples of valid strings:

0106590284
1103487086
1003238829
1713695987
0201564820
1721926390

The implementation I have of this algorithm, I did some time ago at the university, and I would like to see if it can be optimized, either by reducing code, or by implementing other functions, the code is as follows:

function validar(cadena) {
  var cad = $.trim(cadena);
  var par = 0;
  var impar = 0;
  var total = 0;
  var decena = 0;
  var resultado = 0;
  var longitud = cad.length;
  if(cad!=="" && cad.length === 10){
    for(i=0;i<longitud-1;i++){
      if(i%2 === 0){
        impar =0;
        par = cad.charAt(i) * 2;
        if(par > 9 ){
          par = par-9;
        }
      }else{
        par = 0;
        impar = cad.charAt(i) *1;
      }
      total=total+par+impar;
    }
    decena = (Math.floor(total/10)+1)*10;
    resultado = decena-total;
    if (resultado === 10) {
      if (ced.charAt(longitud-1) == 0) {
        alert("Cadena Válida");
      }else{
        alert("Cadena Inválida");
      }
    }else {
      if (cad.charAt(longitud-1) == resultado) {
        alert("Cadena Válida");
      }else{
        alert("Cadena Inválida");
      }
    }

  }
}
<!DOCTYPE html>
<html>
<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width">
  <title>Longitud</title>
</head>
<body>
<input type="text" id="ced">
<button type="button" name="button" onclick="validar($('#ced').val())">Validar</button>
<div id="salida"></div>
<script src="https://code.jquery.com/jquery-3.1.0.js"></script>
</body>
</html>

javascript

2 Answers

  1. Best Answer

    You can simplify the code by doing several things:

    • Reduce unnecessary operations (eg there is no need to multiply by 1 because a number multiplied by 1 will be itself)
    • Simplify complex operations (eg when you calculate the final result, you use an excessively complex function to calculate the upper ten (Math.floor(total/10)+1)*10 - total, when you could do something much simpler like 10 - total % 10... although this has a little more meat for the case in which the number is a multiple of 10, but it is easy to solve).
    • Reduce the number of unnecessary variables and the operations performed on them (eg in the loop you are keeping track of two variables pare imparbut you are really only using one at a time and the other is 0)
    • Eliminate the redundancy of functions and access to parameters that you have already executed/accessed before (eg the lengthone mentioned by eledgaar or the one charAtin the checks).
    • Fix errors and bugs in the code (eg you save the field value cedin cad, but there is a typo and make ced.charAt(...)it work because the browser will fix the bug, but it's not ideal, plus the code might break).
    • Instead of using jQuery, use pure JavaScript. For the little you do, you're loading a JS library and using functions that are super simple in JS without needing anything else.

    Making those code changes:

    function validar() {
  var cad = document.getElementById("ced").value.trim();
  var total = 0;
  var longitud = cad.length;
  var longcheck = longitud - 1;

  if (cad !== "" && longitud === 10){
    for(i = 0; i < longcheck; i++){
      if (i%2 === 0) {
        var aux = cad.charAt(i) * 2;
        if (aux > 9) aux -= 9;
        total += aux;
      } else {
        total += parseInt(cad.charAt(i)); // parseInt o concatenará en lugar de sumar
      }
    }

    // la parte más compleja: si es un múltiplo de 10 será 0, sino, 10-el módulo
    total = total % 10 ? 10 - total % 10 : 0;

    // la condición de arriba te la ahorras abajo, haciendo el código más claro
    if (cad.charAt(longitud-1) == total) {
      alert("Cadena Válida");
    }else{
      alert("Cadena Inválida");
    }
  }
}
    <input type="text" id="ced">
<button type="button" name="button" onclick="validar()">Validar</button>
<div id="salida"></div>

    • 4

  2. In the above condition use longitudinstead of recalculating dad.lengtheach time, and in the loop fordon't subtract 1 from the variable longitud, you can do it at the time you declare it; var longitud = cad.length - 1Or declare another variable for this case. The calculation of the lengthin each iteration of the loop is quite expensive.

    • 1