¿Cuál es la mejor manera de refactorizar este código para limpiarlo?

1) Selecciona de la base de datos agregando una columna para el porcentaje de diferencia entre dos columnas

2) Recorre los valores de las columnas

3) Si la fecha es pasada

4) Si el precio es superior a 500 y la diferencia porcentual es inferior al primer argumento, establezca el indicador en 1

5) De lo contrario, si el precio es inferior a 500 y la diferencia porcentual es inferior al segundo argumento, establezca el indicador en 1

6) De lo contrario, mantenga la bandera como 0

def calculateEmployeeSpend(read_cursor, flag_higher_amount, flag_lower_budget):

    read_cursor.execute("SELECT distinct b.employee_id, b.amount, "
                "s.spend, b.date, b.amount - s.spend as spend_left,  "
                "100.0*(b.amount - s.spend) / b.amount As PercentDiff FROM employee_budget_upload "
                "As b JOIN employee_budget_spent As s ON  b.employee_id = s.employee_id where b.amount != 0")

    for employee_id, amount, spend, date, spend_left, percent_diff in read_cursor:
        flag=0
        date_of_amount = dt.strptime(date, "%d/%m/%Y")
        if date_of_amount <= dt.now():
            if amount > 500 and percent_diff < int(flag_higher_amount):
                flag=1

            if amount < 500 and percent_diff < int(flag_lower_amount):
                flag=1

Editar:

He cambiado los ifs a uno si:

if amount > 500 and percent_diff < int(flag_higher_amount) or amount < 500 and percent_diff < int(flag_lower_amount):
                    flag=1
0
Sam 1 mar. 2018 a las 15:07

3 respuestas

La mejor respuesta
  1. Extraiga el comando SQL en un archivo.sql. Dé a la función la ruta al archivo sql o el texto del archivo sql.
  2. Cambie el nombre de la bandera a su propósito.
  3. tu if y elif establecen la bandera en 1, entonces ¿por qué las diferencias? Combínalo con una condición
  4. '500' debería ser una variable de la función.
  5. Escriba una breve descripción de lo que hace la función en "" "" "". Puede especificar cada parámetro si lo desea.
1
Eran Moshe 1 mar. 2018 a las 12:12

Dado que parece haber varias instancias de hacer un seguimiento del estado, a la larga podría ayudar desacoplar la lógica en clases y métodos.

0
solstice333 1 mar. 2018 a las 12:23

En primer lugar, la definición de "mejor" depende de su objetivo, como: legibilidad, eficiencia, rendimiento, etc.

En muchos casos, preferiría resolver tareas como esta leyendo todo el conjunto de datos en el marco de datos de pandas y utilizar uno (o activar) modismos de pandas convenientes y expresivos. O, escribiendo una declaración SQL más sofisticada que permita resolver la tarea de extremo a extremo en el lado de la base de datos.

En cuanto a las mejores prácticas comunes para la refactorización, recomendaría externalizar los valores mágicos como "500" o "% d /% m /% Y" a un parámetro constante o método. Ponle una "bandera" más nombre auto-hablado. Si el caso con una cantidad exactamente igual a 500 se debe llevar a un indicador igual a cero, entonces debería reflejarse mejor explícitamente en los comentarios. Para evitar la duplicación de código (flag = 1) es mejor combinar las declaraciones if, de esta manera:

if amount > 500 and percent_diff < int(flag_higher_amount) or \
   amount < 500 and percent_diff < int(flag_lower_amount):
    flag=1

También puede crear una función con nombre auto-hablado y mover la condición completa dentro de dicha función:

if is_percent_inside_amount_bounds(
     percent_diff, amount, flag_lower_amount, flag_higher_amount):
    flag = 1

O solo

flag = is_percent_inside_amount_bounds(
     percent_diff, amount, flag_lower_amount, flag_higher_amount)

En caso de que una cantidad igual a 500 exactamente se pueda interpretar como una cantidad <= 500, la condición se podría transformar en más lacónica:

flag = percent_diff < int(
    flag_lower_amount if amount>500 else flag_higher_amount)

Pero no recomendaría usar el operador ternario en el código de producción en casos como este, porque generalmente reduce la legibilidad.

0
Recontemplator 1 mar. 2018 a las 12:46