6

我最近在另一个问题中写的这个特定代码，我不确定它是否是最优的。不过，我找不到一种缩进较少的方法。在那儿？

def msg_generator(self):
    ''' Provides messages until bot dies '''
    while self.alive:
        for msg in self.irc.recv(self.buffer).split(('\r\n').encode()):
            if len(msg) > 3:
                try: 
                    yield Message(msg.decode())
                except Exception as e:
                    self.log('%s %s\n' % (except_str, str(e)))

我一直听说嵌套太多不好，但这似乎是必要的。它目前有四个凹痕。

4

2 回答 2

10

在我的脑海中，你可以这样做：

def msg_generator(self):
    ''' Provides messages until bot dies '''
    while self.alive:
        for msg in self.irc.recv(self.buffer).split(('\r\n').encode()):

            if len(msg) <= 3:
                continue

            try: 
                yield Message(msg.decode())
            except Exception as e:
                self.log('%s %s\n' % (except_str, str(e)))

或者你可以重构为函数。

def msg_generator(self):
    ''' Provides messages until bot dies '''
    while self.alive:
        for msg in self.irc.recv(self.buffer).split(('\r\n').encode()):

            if not len(msg) > 3:
                continue

            yield handle_message(msg)

def handle_message(msg):
    try: 
        return Message(msg.decode())
    except Exception as e:
        self.log('%s %s\n' % (except_str, str(e)))

或者使用这样的东西：

from itertools import imap

def msg_generator(self):
    ''' Provides messages until bot dies '''
    while self.alive:
        it = iter(self.irc.recv(self.buffer).split(('\r\n').encode()))
        return imap(handle_message, (msg for msg in it if len(msg) > 3)

def handle_message(msg):
    try: 
        return Message(msg.decode())
    except Exception as e:
        self.log('%s %s\n' % (except_str, str(e)))

最后一个选项并不完美，因为如果出现异常，则 func 将返回None这不是真正的消息，因此您也可以在之后使用filter()或让另一端处理Nonemsg 来过滤它。

于 2013-10-14T15:19:00.090 回答
4

上面代码的问题与其说是嵌套太深，不如说是代码过于外延，不够刻意。我的意思是，虽然您组装机器所需的齿轮、轮子和杠杆都已就位并且（可能）确实可以工作，但看到的标记模块太少了——在太多隐含的情况下“完成”了太多的问题，未标记的方式，导致难以阅读的代码（对于那个“复杂”部分，你愿意听听http://www.infoq.com/presentations/Simple-Made-Easy - 这是一个非常有启发性的谈话，它指出了您的代码存在的问题）。

让我举个例子：

...
for msg in self.irc.recv(self.buffer).split(('\r\n').encode()):
...

我认为您在这里谈论“消息”。那么为什么不调用那个变量message呢？为什么缩写。那个词？message清楚多了。

现在是for message in xxx。什么是xxx？好吧，你已经知道了。在 Pythonic 英语中，确实，在大多数情况下，这for apple in appples; for pear in pears应该是一种好的、合理的语义书写方式：for ... in循环从元素集合中一次一个地挑出元素，所以for <singular> in <plural>大多数情况下都可以时间。这给了我们

...
for message in messages:
...

这些消息是从哪里来的？根据您的代码，它们是做的结果self.irc.recv(self.buffer).split(('\r\n').encode())。

现在真是太可惜了。对于初学者来说，('\r\n').encode()绝对与 相同'\r\n'.encode()，即少了一级括号。现在这肯定是一个常数，那么为什么要在每一轮重新计算呢？最好对所有缓冲区返回进行解码，irc.recv()而不是先拆分然后逐行解码。

但这是次要的问题。重要的是：无论需要什么低级代码来为您提供来自 的下一个行列表irc，它都不应该出现在这个位置。相反，像

messages = self.irc.get_messages()

或者可能

messages = <SOME_LIBRARY>.get_irc_messages( self.irc )

将是适当的。当然，细节受到许多有争议的讨论。就个人而言，我尽量不将我的数据（主要是泛型类型的有状态对象，没有附加功能）与我的库（命名功能的无状态集合）混为一谈。上面链接的Simple Made Easy演示文稿提出了完全相同的观点，顺便说一句，并且也设法令人信服地争论。（旁白：当然，这违背了当前主流的 OOP 思想——但如果你相信n'importe quois ，请在谷歌上搜索 Yegge在名词王国的死刑。）OOP ≡ good

目前的方法都是关于回复消息，而不是接收消息。把它想象成报纸上的 Yours Truly 专栏：在这里，我们在办公室里思考和汇编答案，然后发送给读者；那些打开收到的纸质邮件、拿起电话并筛选电子邮件的人，在另一个办公室从事着完全不同的工作。因此，如何打开信封不应该是你应该在这个地方实施的事情。需要外包。

您是否要使该messages变量显式或将其合并为内联 API 调用取决于诸如可用空间之类的数字因素以及您是否要在例程的另一点回收该返回值。这对我来说很好：

...
for message in self.irc.get_messages():
...

for ... inwill 透明地与生成器和列表返回器一起工作，这很好。

始终以最通用、最不臃肿、最模块化但不是“原子化”的方式来考虑你的代码。

如果我要针对您的代码编写测试用例，我肯定会挑出那irc.recv部分并单独测试它。以这种方式思考您的代码：什么是好的模块，每个模块都有一个单独的关注点和合理的隔离级别，合理的调用参数，这有助于良好的可测试性？我的代码部分的合理名称是什么？——如果你坚持这一点，你的代码就会停止嵌套太深，完全靠自己。

于 2013-10-14T18:47:14.613 回答