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 等更改为属性。您可能在这里需要一个命名元组，而不是字典。

于 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 项目。这是如何成为最模块化和最优雅的，而不是必要的 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 回答
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 回答