Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactorizando la generación del checksum #14

Closed
wants to merge 5 commits into from

Conversation

fitorec
Copy link

@fitorec fitorec commented Apr 28, 2022

Hola buenas,

Hice pequeñas modificaciones al código, como:

  • Remplazar el diccionario de un arreglo a un string.
  • El antiguo $j originalmente era la longitud mas uno, pero... si extraemos la longitud antes de realizar el pop ya no es necesario sumar la unidad.
  • Finalmente vi que la diferencia del once menos el modulo(11), solamente se utiliza para caso en donde el resultado de la operación cuando el modulo es distinto de cero y uno.

Respecto a la implementación del algoritmo vi esta documentación:

https://www.studocu.com/es-mx/document/universidad-del-valle-de-mexico/administracion/algoritmo-para-generar-el-rfc-con-homoclave-para-personas-fisicas-y-morales/12002840

Pero... me llama la atención que en el anexo I, veo diferencias en la tabla de caracteres(diccionario), no sé si tengas mayor información que pueda revisar y aclarar mis dudas.

Gracias.

@eclipxe13
Copy link
Member

Hola, los algoritmos son similares y producen el mismo resultado:

Remplazar el diccionario de un arreglo a un string.

No te lo recomiendo, en un arreglo el acceso a los caracteres es muy rápido, la función strpos es lenta. Probablemente entra como "micro-optimización", pero al usarla en una gran cantidad de RFC (como la LRFC) es una gran diferencia.

El antiguo $j originalmente era la longitud mas uno, pero... si extraemos la longitud antes de realizar el pop ya no es necesario sumar la unidad.

Sí, también se puede hacer de esta forma.

Finalmente vi que la diferencia del once menos el modulo(11), solamente se utiliza para caso en donde el resultado de la operación cuando el modulo es distinto de cero y uno.

Sí, también se puede hacer de esta forma.

Pero... me llama la atención que en el anexo I, veo diferencias en la tabla de caracteres(diccionario), no sé si tengas mayor información que pueda revisar y aclarar mis dudas.

El caracter Ñ es multi-byte, por ejemplo strlen('Ñ') === 2, por eso es mejor trabajar con un caracter que no sea multi-byte y es sustituido por #.

Con respecto al PR, ve la documentación de CONTRIBUTING.md:

  • strpos puede devolver false.
tools/psalm --no-progress

ERROR: PossiblyFalseOperand - src/CheckSum.php:21:21 - Left operand cannot be falsable, got false|int|positive-int (see https://psalm.dev/162)
            $sum += $posChar * $factor;
  • Hay algunos errores de code style, se puede ejecutar para corregir:
composer update
phive update
composer dev:build

Por último, y lo más importante:
La forma de obtener el checksum en tu PR es igual (equivalente) a la forma que está escrita, ligeramente menos eficiente dado el uso de strpos. ¿Cuál sería la razón para modificar el algoritmo actual?

@fitorec
Copy link
Author

fitorec commented Apr 28, 2022

Hola, los algoritmos son similares y producen el mismo resultado:

Remplazar el diccionario de un arreglo a un string.

No te lo recomiendo, en un arreglo el acceso a los caracteres es muy rápido, la función strpos es lenta. Probablemente entra como "micro-optimización", pero al usarla en una gran cantidad de RFC (como la LRFC) es una gran diferencia.

Buenas acabé de hacer una mezcla de ambas complementaciones dejando la generación del arreglo del diccionario dentro del constructor, así una vez creado el objeto se podrá calcular las veces que se desee el checksum accediendo al mismo diccionario.

Pero... me llama la atención que en el anexo I, veo diferencias en la tabla de caracteres(diccionario), no sé si tengas mayor información que pueda revisar y aclarar mis dudas.

El caracter Ñ es multi-byte, por ejemplo strlen('Ñ') === 2, por eso es mejor trabajar con un caracter que no sea multi-byte y es sustituido por #.

Me queda claro.

Con respecto al PR, ve la documentación de CONTRIBUTING.md:

Sí, gracias lo acabé de revisar.

  • Hay algunos errores de code style, se puede ejecutar para corregir:
    ....

Revisado.

Por último, y lo más importante: La forma de obtener el checksum en tu PR es igual (equivalente) a la forma que está escrita, ligeramente menos eficiente dado el uso de strpos. ¿Cuál sería la razón para modificar el algoritmo actual?

En primera te agradezco el tiempo prestado, la verdad es que de un tiempo acá estoy trabajando para empresas financieras mexicanas, mi trabajo es en la web, sin embargo los backends que estoy haciendo lo desarrollo en python por eso me es importante conocer lo que transiberianas hacen estos algoritmos.

En este sentido estoy revisando distintas complementaciones y documentaciones sobre determinados algoritmos que ayuden a diferentes tramites en México y en algunos casos estoy haciendo algunas implementaciones, por ejemplo:

https://gist.github.com/fitorec/2c221e3314e6f3e7f87216c8d4762d0c

Sin embargo del checksum generado para los RFCs encontré poca información (salvo el documento que te compartí).

Es por este motivo que realicé un fork sobre este proyecto con la finalidad de aprender mas hacer de dicho algoritmo, pero... como no me gusta pedir sin dar prefiero contribuir para que el karma sea devuelto 😄.

Por lo cual creo que mis contribuciones van enfocadas en hacer mas entendible dicho algoritmo hacia otras personas que en su momento deseen contribuir o simplemente entender como funciona este algoritmo del checksum.

@eclipxe13
Copy link
Member

Sin embargo del checksum generado para los RFCs encontré poca información (salvo el documento que te compartí).

Ok, probablemente te gustaría saber que el checksum no se cumple ni se sigue en la práctica (sí, así es el SAT).
Está el caso del RFC RT0840921RE4 de la cadena de restaurantes TOKS, donde termina en 4 pero el debería ser A, incluso, alguna vez sí fue RT0840921REA.

Por lo cual creo que mis contribuciones van enfocadas en hacer mas entendible dicho algoritmo hacia otras personas que en su momento deseen contribuir o simplemente entender como funciona este algoritmo del checksum.

Entiendo, y gracias por tu esfuerzo.
En PHP es mejor tener el arreglo como una constante que en una variable estática.
Pero, siguiendo tu implementación, el código para llenar la variable estática debería ser algo así:

    /** @var int[] **/
    private static $dictionary = [];

    public function __construct()
    {
        if ([] === self::$dictionary) {
            self::$dictionary = array_flip(str_split('0123456789ABCDEFGHIJKLMN&OPQRSTUVWXYZ #'));
        }
    }

Voy a dejar la lógica del algoritmo como está actualmente, con unas pequeñas modificaciones:

  • En comentarios la liga que pusiste (que también la conocía pero vale la pena documentarla).
  • Mejorando el uso de$length.
  • Mejorando el test para que compruebe estos casos personas físicas y morales, que teminen en 0, 9 o [1-9], que contengan multibyte e inválidos (cadenas vacías, cadenas que usan caracteres no válidos)...

Reitero, muchas gracias por tu trabajo.

@eclipxe13 eclipxe13 closed this Apr 28, 2022
@eclipxe13 eclipxe13 mentioned this pull request Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants