留下“其他”评论

Question

假设我的代码中有一个不那么直观的if语句，但前提是您对代码库还不熟悉：

def set_markets(markets=None):
    """
    Will accept 'all' as markets to set all markets on.
    For individual markets, pass in a list of ints.
    """
    if markets:
        if type(markets) == list:
            markets = ','.join(map(str, markets))
        self._set_markets(markets)

对于那些想知道：

我正在与之交互的现有系统(数据库)需要一个来自前端接口的整数列表。这段代码是从后端执行的，因此选择了一个ints列表，因为它最能代表用户可能输入的内容。

markets的东西是从另一个类传下来的，所以即使它看起来像发生在5行以上，它更像~50行。

我的问题是，你会认为这更易读吗？为什么或者为什么不？

def set_markets(markets=None):
    """
    Will accept 'all' as markets to set all markets on.
    For individual markets, pass in a list of ints.
    """
    if markets:
        if type(markets) == list:
            markets = ','.join(map(str, markets))
      # else:
          # markets are set to "all"
        self._set_markets(markets)

编辑：

这段代码看起来比它更糟，因为我忽略了一个细节:如果None是为markets (默认参数)提供的，那么它将不会修改市场设置数据库。

我现在拥有的是：

    if markets is not None:
        if isinstance(markets, list):
            markets = ','.join(map(str, markets))
        else:
            markets = 'all'
        self._set_new_header_markets(markets)

Answer 1

首先，在注释中添加代码使我假设它是死代码。如果你这么做的话会很混乱的。

第二，您需要注释是因为您的代码不可读。尝试在添加注释之前使代码可读性。

如果你接受“全部”或一个列表，你真的应该这样做：

if markets == 'all':
   self._set_markets('all')
else:
   self._set_markets(','.join(map(str, markers))

然后，它非常清楚发生了什么，避免了类型检查，并且只要数据不是列表，就不会接受无效的数据。

Answer 2

对我来说，将else:格式化为像普通代码这样的事实是有误导性的；更常见的事情似乎是合适的。

现在，其他代码似乎被注释掉了，因为还没有该块的代码，内部注释是代码的占位符，而不是(?)。我会将这两个注释内联起来，在散列上突出缩进，并可能删除其他的格式：

if markets:
    if type(markets) == list:
        markets = ','.join(map(str, markets))
    # else: markets are set to "all"
    self._set_markets(markets)