2009-09-02 6 views
2

Je me demandais s'il y avait des moyens de simplifier le morceau de code suivant. Comme vous pouvez le voir, il existe de nombreux dicts utilisés ainsi que des déclarations de conditions pour éliminer les mauvaises données d'entrée. Notez que les valeurs de taux de voyage ne sont pas encore tous inputed, les dicts sont simplement copiés et collés pour l'instantsimplification des structures de données et des instructions de condition dans le code python

EDIT

Dans tous les taux, (x, y): z. x et y sont corrects, les valeurs z ne sont pas car ils sont tout simplement copier/coller

ce code fonctionne dans le cas où vous souhaitez copier, coller et tester

import math 


# step 1.4 return trip rates 
def trip_rates(population_stratification, analysis_type, low_income, medium_income, high_income): 
    ''' this function returns the proper trip rate tuple to be used based on input 
    data 
    ADPT = Average Daily Person Trips per Household 
    pph = person per household 
    veh_hh = vehicles per household 
    (param_1, param_2): ADPT 
    ''' 
    li = low_income 
    mi = medium_income 
    hi = high_income 
    # table 5 - 
    if analysis_type == 1: 
    if population_stratification == 1: 
     rates = {(li, 1):3.6, (li, 2):6.5, (li, 3):9.1, (li, 4):11.5, (li, 5): 13.8, 
       (mi, 1):3.9, (mi, 2):7.3, (mi, 3):10.0, (mi, 4):13.1, (mi, 5): 15.9, 
       (hi, 1):4.5, (mi, 2):9.2, (mi, 3):12.2, (mi, 4):14.8, (mi, 5): 18.2} 
     return rates 
    if population_stratification == 2: 
     rates = { 
       (li, 1):3.1, (li, 2):6.3, (li, 3):9.4, (li, 4):12.5, (li, 5): 14.7, 
       (mi, 1):4.8, (mi, 2):7.2, (mi, 3):10.1, (mi, 4):13.3, (mi, 5): 15.5, 
       (hi, 1):4.9, (mi, 2):7.7, (mi, 3):12.5, (mi, 4):13.8, (mi, 5): 16.7 
       } 
     return rates 
    if population_stratification == 3: #TODO: input actual rate 
     rates = { 
       (li, 1):3.6, (li, 2):6.5, (li, 3):9.1, (li, 4):11.5, (li, 5): 13.8, 
       (mi, 1):3.9, (mi, 2):7.3, (mi, 3):10.0, (mi, 4):13.1, (mi, 5): 15.9, 
       (hi, 1):4.5, (mi, 2):9.2, (mi, 3):12.2, (mi, 4):14.8, (mi, 5): 18.2 
       } 
     return rates 
    if population_stratification == 4: #TODO: input actual rate 
     rates = { 
       (li, 1):3.1, (li, 2):6.3, (li, 3):9.4, (li, 4):12.5, (li, 5): 14.7, 
       (mi, 1):4.8, (mi, 2):7.2, (mi, 3):10.1, (mi, 4):13.3, (mi, 5): 15.5, 
       (hi, 1):4.9, (mi, 2):7.7, (mi, 3):12.5, (mi, 4):13.8, (mi, 5): 16.7 
       } 
     return rates 
    #table 6 
    elif analysis_type == 2: 
    if population_stratification == 1: #TODO: Change rates 
     rates = { 
       (0, 1):3.6, (0, 2):6.5, (0, 3):9.1, (0, 4):11.5, (0, 5): 13.8, 
       (1, 1):3.9, (1, 2):7.3, (1, 3):10.0, (1, 4):13.1, (1, 5): 15.9, 
       (2, 1):4.5, (2, 2):9.2, (2, 3):12.2, (2, 4):14.8, (2, 5): 18.2, 
       (3, 1):4.5, (3, 2):9.2, (3, 3):12.2, (3, 4):14.8, (3, 5): 18.2 
       } 
     return rates 
    if population_stratification == 2: #TODO: Change rates 
     rates = { 
       (0, 1):3.6, (0, 2):6.5, (0, 3):9.1, (0, 4):11.5, (0, 5): 13.8, 
       (1, 1):3.9, (1, 2):7.3, (1, 3):10.0, (1, 4):13.1, (1, 5): 15.9, 
       (2, 1):4.5, (2, 2):9.2, (2, 3):12.2, (2, 4):14.8, (2, 5): 18.2, 
       (3, 1):4.5, (3, 2):9.2, (3, 3):12.2, (3, 4):14.8, (3, 5): 18.2 
       } 
     return rates 
    if population_stratification == 3: #TODO: Change rates 
     rates = { 
       (0, 1):3.6, (0, 2):6.5, (0, 3):9.1, (0, 4):11.5, (0, 5): 13.8, 
       (1, 1):3.9, (1, 2):7.3, (1, 3):10.0, (1, 4):13.1, (1, 5): 15.9, 
       (2, 1):4.5, (2, 2):9.2, (2, 3):12.2, (2, 4):14.8, (2, 5): 18.2, 
       (3, 1):4.5, (3, 2):9.2, (3, 3):12.2, (3, 4):14.8, (3, 5): 18.2 
       } 
     return rates 
    if population_stratification == 4: #TODO: Change rates 
     rates = { 
       (0, 1):3.6, (0, 2):6.5, (0, 3):9.1, (0, 4):11.5, (0, 5): 13.8, 
       (1, 1):3.9, (1, 2):7.3, (1, 3):10.0, (1, 4):13.1, (1, 5): 15.9, 
       (2, 1):4.5, (2, 2):9.2, (2, 3):12.2, (2, 4):14.8, (2, 5): 18.2, 
       (3, 1):4.5, (3, 2):9.2, (3, 3):12.2, (3, 4):14.8, (3, 5): 18.2 
       } 
     return rates 
    # table 7 
    elif analysis_type == 3: 
    if population_stratification == 1: #TODO: input actual rate 
     rates = { 
       (li, 0):3.6, (li, 1):6.5, (li, 2):9.1, (li, 3):11.5, 
       (mi, 0):3.9, (mi, 1):7.3, (mi, 2):10.0, (mi, 3):13.1, 
       (hi, 0):4.5, (mi, 1):9.2, (mi, 2):12.2, (mi, 3):14.8 
       } 
     return rates 
    if population_stratification == 2: #TODO: input actual rate 
     rates = { 
       (li, 0):3.6, (li, 1):6.5, (li, 2):9.1, (li, 3):11.5, 
       (mi, 0):3.9, (mi, 1):7.3, (mi, 2):10.0, (mi, 3):13.1, 
       (hi, 0):4.5, (mi, 1):9.2, (mi, 2):12.2, (mi, 3):14.8 
       } 
     return rates 
    if population_stratification == 3: #TODO: input actual rate 
     rates = { 
       (li, 0):3.6, (li, 1):6.5, (li, 2):9.1, (li, 3):11.5, 
       (mi, 0):3.9, (mi, 1):7.3, (mi, 2):10.0, (mi, 3):13.1, 
       (hi, 0):4.5, (mi, 1):9.2, (mi, 2):12.2, (mi, 3):14.8 
       } 
     return rates 
    if population_stratification == 4: #TODO: input actual rate 
     rates = { 
       (li, 0):3.6, (li, 1):6.5, (li, 2):9.1, (li, 3):11.5, 
       (mi, 0):3.9, (mi, 1):7.3, (mi, 2):10.0, (mi, 3):13.1, 
       (hi, 0):4.5, (mi, 1):9.2, (mi, 2):12.2, (mi, 3):14.8 
       } 
     return rates 

def interpolate(population_stratification, analysis_type, low_income, medium_income, high_income, x, y): 
    #get rates dict 
    rates = trip_rates(population_stratification, analysis_type, low_income, medium_income, high_income) 


    # dealing with x parameters 
    #when using income levels, x_1 and x_2 are li, mi, or hi 
    if analysis_type == 1 or analysis_type == 2 or analsis_type == 4: 
    if x < high_income and x >= medium_income: 
     x_1 = medium_income 
     x_2 = high_income 
    elif x < medium_income: 
     x_1 = low_income 
     x_2 = medium_income 
    else: 
     x_1 = high_income 
     x_2 = high_income 
    if analysis_type == 3: 
    if x >= 3: 
     x_1 = 3 
     x_2 = 3 
    else: 
     x_1 = int(math.floor(x)) 
     x_2 = int(math.ceil(x)) 

    # dealing with y parametrs 
    #when using persons per household, max number y = 5 
    if analysis_type == 1 or analysis_type == 4: 
    if y >= 5: 
     y_1 = 5 
     y_2 = 5 
    else: 
     y_1 = int(math.floor(y)) 
     y_2 = int(math.ceil(y)) 
    elif analysis_type == 2 or analysis_type == 3: 
    if y >= 5: 
     y_1 = 5 
     y_2 = 5 
    else: 
     y_1 = int(math.floor(y)) 
     y_2 = int(math.ceil(y)) 

    # denominator 
    z = ((x_2 - x_1) * (y_2 - y_1)) 

    result = (((rates[(x_1, y_1)]) * ((x_2 - x) * (y_2 - y))/(z)) + 
      ((rates[(x_2, y_1)]) * ((x - x_1) * (y_2 - y))/(z)) + 
      ((rates[(x_1, y_2)]) * ((x_2 - x) * (y - y_1))/(z)) + 
      ((rates[(x_2, y_2)]) * ((x - x_1) * (y - y_1))/(z))) 

    return result 

#test 
low_income = 20000 #this is calculated using exchange rates 
medium_income = 40000 # this is calculated using exchange rates 
high_income = 60000 # this is calculated using exchange rates 
population_stratification = 1 #inputed by user 
analysis_type = 1 #inputed by user 
x = 35234.34 #test income 
y = 3.5 # test pph 

print interpolate(population_stratification, analysis_type, low_income, medium_income, high_income, x, y) 

Répondre

5

Eh bien, où commencer ? Voici juste une première observation:

Vous avez beaucoup de données là-bas, et il semble que le code et les données sont mélangés les uns aux autres.

Les données et le code doivent être séparés. Les données sont une source externe, quelque chose que vous modifiez ou lisez. Vous pourriez probablement adapter votre code pour analyser rapidement les données d'une bonne représentation éditable à une représentation utile pour vos algorithmes. Je soupçonne que votre code sera plus court, plus clair et moins sujette aux erreurs (avez-vous remarqué que tous les dictionnaires «taux» ont plusieurs clés, et vous manquez beaucoup de «salut» clés?).

Si vous avez besoin de meilleures abstractions telles que les matrices et les tableaux de données, regarder dans numpy


Modifier 1

Avez-vous compté le nombre de vos dimensions? Vous avez ici une matrice multidimensionnelle avec des dimensions X: type_analyse, population_stratification, niveau_sommaire, index

Si je vois bien, il s'agit d'une "matrice" 3x4x3x3 (= 108 entrées) ou d'une "table de correspondance". Si ce sont les données sur lesquelles votre modèle s'appuie, très bien. Mais ne pouvez-vous pas mettre ces chiffres dans un fichier ou dans une table que vous avez lue? Votre code serait à côté de trivial.


Edit 2

Ok, je vais mordre pour un style python mineur: Test des valeurs dans un ensemble ou une plage.

Au lieu de:

if analysis_type == 1 or analysis_type == 2 or analsis_type == 4: 

vous pouvez utiliser

if analysis_type in (1, 2, 4): 

ou même en utilisant des noms lisibles comme (CUBES, ..) comme l'a suggéré.

Au lieu de:

if x < high_income and x >= medium_income: 

vous pouvez utiliser des conditions enchaînées; Python est l'un des rares langages de programmation où la chaîne de conditions pour faire nautral si les déclarations:

if medium_income <= x < high_income: 

Modifier 3

Plus importantes que les petits chiffres de code est la conception de code de cours et refactoring. Edit 2 ne peut que vous donner du vernis.

Vous devriez apprendre à détester le code en double.

De plus, vous avez beaucoup de branches dans une fonction. C'est un bon signe que vous devriez le diviser en plusieurs fonctions. Cela peut également réduire la duplication. Par exemple, quand une variable comme analysis_type peut totalement changer ce que la fonction fait, pourquoi avoir deux comportements différents dans une fonction? Vous ne devriez pas avoir le programme entier dans une fonction. Peut-être que analysis_type == 3 est mieux exprimé dans sa propre fonction (à titre d'exemple)? Comprenez-vous que votre fonction trip_rates fait essentiellement une recherche de tableau, où la recherche de tableau est codée en dur comme si ..: return .. if: return .., et le tableau est écrit en entier dans la fonction? Et si trip_rates pouvait être implémenté comme ça? Serait-il possible?

data_model = compute_table(low_income, ...) 
return data_model[analysis_type][population_stratification] 
+0

Je ai apparié indexé pour faciliter l'interpolation Je vais résoudre le problème «salut», merci de le signaler – dassouki

+1

Je suis sûr qu'il y a beaucoup à ce code que je ne comprends pas. pourquoi vous mettez les niveaux de revenu dans les clés du dictionnaire, mais je ne vais pas lire beaucoup plus, car le code est illisible.Factor les données du code! – u0b34a0f6ae

+0

bien bas/moyen/élevé/niveaux de revenus sont générés sur la base de entrée de l'utilisateur, comme la monnaie, taux de change, et d'autres facteurs. – dassouki

2

Outre la suggestion de Kaizer sur les données et le code, voici quelques opérations de nettoyage simples:

Le code

if y >= 5: 
     y_1 = 5 
     y_2 = 5 
    else: 
     y_1 = int(math.floor(y)) 
     y_2 = int(math.ceil(y)) 

peut être écrit comme

min(5, int(math.floor(y)) 

ou

int(math.floor(min(5, y)) 

ou même fait une fonction:

def limitedInt(v, maxV): 
    return min(5, int(math.floor(y)) 

Aussi, je recommande que, au lieu de dire analysis_type == 1 vous dites quelque chose comme analysis_type = CUBIC (c.-à-un nom qui décrit le type d'analyse) et définir le nom à 1. Cela ne simplifiera pas tellement que de rendre le code plus facile à lire.

Vous trouverez peut-être le livre Refactoring par Martin Fowler ou Refactoring Workbook par William Wake comme un moyen d'en savoir plus sur le nettoyage du code (the website est également disponible, mais sans savoir à propos de « code sent » décrit dans les livres, il est pas

+0

merci, je vais chercher dans l'achat du livre. – dassouki

+0

et merci pour les snippets – dassouki

+0

Les fonctions 'max' et' min' sont en fait intégrées, alors appelez les 'min (..)' – u0b34a0f6ae