2012-04-10 1 views
0

Ich habe eine sehr böse Stück Code, den ich gerne umgestalten würde, aber ich habe, weil es total unlesbar wird.Wie kann ich die Lesbarkeit von verschachtelten if und für Aussagen verbessern

for region in feed['config']['regions']: 
      if region['region'] == region_name: 
       for instance_type in region['instanceTypes']: 
        if instance_type['type'] == instance_type_name: 
         for instance_size in instance_type['sizes']: 
          if instance_size['size'] == instance_size_name: 
           for platform in instance_size['valueColumns']: 
            if platform['name'] == platform_name: 
             prices = platform['prices'] 
             assert prices.keys() == ['USD'] 
             return decimal.Decimal(prices['USD']) 
     assert False, "Failed to determine price for instance with region=%r, type=%r, size=%r, platform=%r" % \ 
       (region_name, instance_type_name, instance_size_name, platform_name) 

Ich habe gelernt über Funktionen mit jeder Schleife oder if-Anweisung, aber das wird mir eine Menge von Funktionen geben. Gibt es bessere Lösungen?

+5

Funktionen verwenden, so y ou haben nicht mehr als 2 verschachtelte for-loops gilt allgemein als guter Stil. "Eine Ladung von Funktionen" ist eine gute Sache (tm), besonders wenn Sie Docstrings verwenden, um sie zu dokumentieren :-) – thebjorn

+1

@thebjorn sollte Ihr Kommentar eine Antwort sein –

Antwort

6
try: 
    region = [r for r in feed['config']['regions'] if region['region'] == region_name][0] 
    instance_type = [t for t in region['instanceTypes'] if i['type'] == instance_type_name][0] 
    # ... 
    return decimal.Decimal(prices['USD']) 
except IndexError: 
    raise Exception("Failed to determine price for instance with region=%r, type=%r, size=%r, platform=%r" % 
       (region_name, instance_type_name, instance_size_name, platform_name)) 

oder einen Schritt weiter:

def filter_by_key(key, value, objects): 
    return [o for o in objects if region[key] == value][0] 

try: 
    region = filter_by_key('region', region_name, feed['config']['regions']) 
    instance_type = filter_by_key('type', instace_type_name, region['instanceTypes']) 
    # ... 
    return decimal.Decimal(prices['USD']) 
except IndexError: 
    raise Exception("Failed to determine price for instance with region=%r, type=%r, size=%r, platform=%r" % 
       (region_name, instance_type_name, instance_size_name, platform_name)) 
2

Wenn Sie wollen es in einer Schleife halten Sie continue verwenden könnten Einzüge sparen:

for region in feed['config']['regions']: 
    if region['region'] != region_name: continue 
    for instance_type in region['instanceTypes']: 
     if instance_type['type'] != instance_type_name: continue 
     for instance_size in instance_type['sizes']: 
      if instance_size['size'] != instance_size_name: continue 
      for platform in instance_size['valueColumns']: 
       if platform['name'] != platform_name: continue 
       prices = platform['prices'] 
       assert prices.keys() == ['USD'] 
       return decimal.Decimal(prices['USD']) 
assert False, "Failed to determine price for instance with region=%r, type=%r, size=%r, platform=%r" % \ 
       (region_name, instance_type_name, instance_size_name, platform_name) 

Oder mit Listenkomprehensionen:

regions = (r for r in feed['config']['regions'] if r['region'] == region_name) 
instance_types = (i for i in r['instanceTypes'] for r in regions if i['type'] == instance_type_name) 
instance_sizes = (i for i in t['sizes'] for t in instance_types if i['size'] == instance_size_name) 
platforms = (p for p in s['valueColumns'] for s in instance_sizes if p['name'] == platform_name) 
for platform in platforms: 
    prices = platform['prices'] 
    assert prices.keys() == ['USD'] 
    return decimal.Decimal(prices['USD']) 
assert False, "Failed to determine price for instance with region=%r, type=%r, size=%r, platform=%r" % \ 
       (region_name, instance_type_name, instance_size_name, platform_name)