0

私はこの醜いコードを持っています。

 orders = got_something_from_db()
 if orders:
    for order in orders:
        #print order
        event_id = order["event_id"]
        if event_id in event_id_dict: # something i grabbed earlier
            product_id = order["product_id"] # products in an event
            qty = order["qty"]
            if product_id in product_sku_dict: 
                sku_id =product_sku_dict[product_id]
                for i in range(qty):
                    sku_ids.append(sku_id)

これをもっとpythonic（そして簡潔に）にするにはどうすればよいですか？

4

5 に答える 5

4

まず、コードは必ずしも悪いものではありません。読んで理解するのに十分明確であるため、保守性の最初のハードル以上のものを確実に通過しました。

問題がある場合、それは深い入れ子です。私はこれをいくつかの機能を作成することによって分割します。それぞれの機能には、それぞれの名前で表現された明確なビジネス目的があります。

def order_has_valid_event(order):
    """Returns True if the order was raised by an event
    in event_id_dict"""
    event_id = order["event_id"]
    return event_id in event_id_dict # something i grabbed earlier

def get_sku_from_order(order):
    """Return the SKU of the product in an order"""
    product_id = order['product_id']
    try:
        return product_sku_dict[product_id]
    except KeyError:
        raise KeyError("Product {0} is not in SKU dictionary".format(product_id))

def get_order_skus(order):
    """Returns a list of SKUs in an order"""
    sku_id = get_sku_from_order(order)
    qty = order["qty"]
    return [sku_id] * qty

# Modify got_something_from_db to always return a list, even if empty 
orders = got_something_from_db()
for order in orders:
    #print order
    if order_has_valid_event(order):
        try:
            sku_ids.extend(get_order_skus(order))
        except KeyError:
            continue

名前が意図と完全に一致しない場合は、この試みを許してください。名前を適切に変更してください。

私が試みた改善：

  • ループqty時間を追加する代わりに、リストを乗算して要素のリストを作成できますqty。['a'] * 3 = ['a'、'a'、'a']
  • ビジネスレベルでは、製品IDがSKUディクショナリにない場合に、黙って無視されるエラーに疑問を投げかけます。これは実際の問題である可能性が高いです（私が働いていた業界では、すべての製品にSKUが必要です）。その場合は、エラーが迅速かつ「大声で」伝播して、適切なエラーハンドラー、または注文が拒否される可能性のある場所に送られます。これをより明確にサポートするために、私はプレーンをキャッチしKeyError、get_sku_from_orderより明示的な例外メッセージをスローしました。
  • got_something_from_dbを変更して、空の場合でも常にリストを返すようにします。これにより、ロジックとフローが簡素化されます
于 2012-08-01T01:02:11.767 に答える
2

限られた文脈で、これが私ができる最善のことです。

orders = got_something_from_db()
for order in orders: #make got something return empty iterable on failure
    if order["event_id"] in event_id_dict:
        product_id = order["product_id"]
        try:
            sku_id = product_sku_dict[product_id]
            #Change sku_ids to a collections.Counter (assuming order is unimportant)
            sku_ids[sku_id] += order["qty"] 
        except KeyError:
            pass

また、event_id、product_idなどを属性に変更することを検討してください。おそらく、ここではdictではなくnamedtupleが必要です。

于 2012-08-01T00:28:17.320 に答える
2

私はそれを次のように構成します：

def add_product_to_skus(product_id, qty):
    if product_id in product_sku_dict: 
        sku_id = product_sku_dict[product_id]
        sku_ids.extend(qty*[sku_id])    

# ...

orders = got_something_from_db()
if not orders:
    return

valid_orders = (o for o in orders if o['event_id'] in event_id_dict)

for o in valid_orders:
    add_product_to_skus(o['product_id'], o['qty'])

上記のいくつかのゴルフはあなたに与えるでしょう：

orders = got_something_from_db()
if not orders:
    return
add_products_to_skus((o['product_id'], o['qty']) for o in orders 
                     if o['event_id'] in event_id_dict 
                     if o['product_id'] in product_sku_dict)
# ...  
def add_product_to_skus(product_qtys):
    for product_id, qty in product_qtys:
        sku_id = product_sku_dict[product_id]
        sku_ids.extend(qty*[sku_id])

ただし、元の形式からこの形式への変換は必ずしも明確（または正しい）ではなく、リスト内包表記はおそらくフィルタリングを説明するコメントに値するでしょう。

于 2012-08-01T00:32:24.523 に答える
1 
sku_ids = []
event_ids = get_event_id_dict()
orders = got_something_from_db()

for order in orders:
    if order["event_id"] in event_ids:
        try:
            sku_id = product_sku_dict[order["product_id"]]
        except KeyError:
            continue

        sku_ids.extend([sku_id] * order["qty"])
于 2012-08-01T00:25:41.463 に答える
1

不足しているSKUアイテムを黙って無視するのではなく、例外を発生させることをお勧めします。これは、最もモジュール化されたエレガントな方法であり、必要なpythonicではありません。

database = ...
productId_to_sku = {...}

def productIdToSku(product_id):
    try:
        return productId_to_sku[product_id]
    except IndexError:
        raise Exception('product id #{} not in SKU database; have a manager add it'.format(product_id))

def getSkusFromEvent(event_id):
    orders = database.fetch(event_id=event_id)
    for order in orders:
        yield (productIdToSku(order.PRODUCT_ID), order.QTY)

collections.Counter(getSkusFromEvent(YOUR_EVENT_ID))
于 2012-08-01T00:34:42.047 に答える