Review: Needs Fixing
Buenas, Oihane,
Gracias por la contribución. Realizando una revisión de código, hay varias
cosas que se podrían hacer para mejorarlo (te indico línea del diff como
referencia):
- l.74: la comparación de las cantidades transformándolas a cadena no es el
mejor método y además puede dar falsos negativos. Aunque ya sé que ese código
ya estaba, puedes aprovechar para mejorarlo cogiendo una instancia de
res.currency para la moneda de la compañía y llamar al método "is_zero" (o
alguien así, que hablo de memoria), restando ambas cantidades, para que te
devuelva correctamente la comparación.
- l.85 y l.89: Puesto que en ambos objetos se escriben los mismos datos, lo que
puedes hacer para simplificar código y que sea más legible es lo siguiente:
if invoice.type=="out_invoice" or invoice.type=="out_refund":
obj = invoices340
else:
obj = invoices340_rec
obj.write(cr, uid, invoice_created,
{'amount_tax': tot_tax_invoice,
'warning': warning,
'warn_message': error})
- En todo el código: por favor, intenta aplicar PEP8 en el código que
modifiques/insertes. De esta forma, se homogeneiza el estilo de programación en
todo el repositorio. Puedes consultar aquí las reglas de este estilo:
http://legacy.python.org/dev/peps/pep-0008/
- l.98-100, l.106-109: En lugar de comentar esas líneas, mejor eliminarlas
directamente, ya que así en el diff se lee mucho mejor y no queda código basura.
Por último, debido al cambio de orientación, debería ampliarse la funcionalidad
con las siguientes características:
- Avisar al terminar el cálculo de que hay algún error en las líneas.
- Comprobar antes de confirmar la declaración si queda alguna línea con
warning, ya que de otro modo, alguien podría seguir adelante con la declaración
y emitirla errónea.
- Permitir eliminar a mano el warning.
Un saludo.
Un saludo.
--
https://code.launchpad.net/~avanzosc/openerp-spain/6.1/+merge/211521
Your team Avanzosc Developers is subscribed to branch
lp:~avanzosc/openerp-spain/6.1.
--
Mailing list: https://launchpad.net/~avanzosc
Post to : [email protected]
Unsubscribe : https://launchpad.net/~avanzosc
More help : https://help.launchpad.net/ListHelp
