Skip to content

take advantage of types and other rust features to refactor#8

Merged
edfloreshz merged 3 commits intoedfloreshz:mainfrom
categulario:categulario/heavy-refactor
Aug 31, 2021
Merged

take advantage of types and other rust features to refactor#8
edfloreshz merged 3 commits intoedfloreshz:mainfrom
categulario:categulario/heavy-refactor

Conversation

@categulario
Copy link
Contributor

Este PR no es necesariamente para hacérsele merge (aunque probablemente sea buena idea). Es más que nada una exploración de lo que podría ser una estructura factible. Los detalles importantes se conservan, pero el flujo en general cambia:

  • exploré un poco la idea de usar funciones libres en lugar de funciones asociadas a la estructura CrateInfo. Al final de cuentas no hay realmente ninguna razón para que estas funciones le "pertenezcan" a CrateInfo, son solamente funciones que hacen cosas.
  • Ya no hay ni un solo clone. Para esto se usan dos cosas. La primera es el hecho de que el ArgMatches almacena referencias a los argumentos pasados por línea de comandos, que tienen un tiempo de vida 'static. La segunda es std::borrow::Cow pues la versión puede venir de dos lugares diferentes: la linea de comandos y el Cargo.toml,
  • Se explora un poco el uso de Option. Por ejemplo para la versión y el query, estos son tipos de datos fundamentalmente opcionales. Pueden o no estar, así que Option es una buena forma de manejarlos en vez de un string vacío o no.
  • Se explora un poco el uso de enums. La documentación que se abre es de una de tres fuentes, y no pueden ocurrir dos al mismo tiempo. Eso es una oportunidad perfecta para meter un enum, que encodea en el sistema de tipos la imposibilidad de tener dos escenarios traslapándose.

y bueno, escribí este código mientras estaba en una junta aburrida. Ahora si que "en mi máquina funciona", jeje. Como decía arriba, no pretendo que se le haga merge, son un chingo de cambios. Más bien es para mostrar otra forma posible de atacar este problema.

Saludos!

categulario and others added 3 commits August 30, 2021 19:40
Agregue la opcion de especificar la version en la biblioteca estandar.
Modifique algunos nombres de variables para hacerlos mas explicitos.
@edfloreshz edfloreshz merged commit a215b77 into edfloreshz:main Aug 31, 2021
@edfloreshz
Copy link
Owner

En general me parece un muy buen aporte, solo corregí un detalle, omitiste el que la biblioteca estándar también puede recibir una versión, fuera de eso me pareció perfecto, gracias 👍🏼

@categulario
Copy link
Contributor Author

categulario commented Aug 31, 2021 via email

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