1

嗨，伙计们，你能帮我重构一下，让它看起来像 Python 一样。

import sys
import poplib
import string
import StringIO, rfc822
import datetime
import logging

def _dump_pop_emails(self):
    self.logger.info("open pop account %s with username: %s" % (self.account[0], self.account[1]))
    self.popinstance = poplib.POP3(self.account[0])
    self.logger.info(self.popinstance.getwelcome()) 
    self.popinstance.user(self.account[1])
    self.popinstance.pass_(self.account[2])
    try:
        (numMsgs, totalSize) = self.popinstance.stat()
        for thisNum in range(1, numMsgs+1):
            (server_msg, body, octets) = self.popinstance.retr(thisNum)
            text = string.join(body, '\n')
            mesg = StringIO.StringIO(text)                               
            msg = rfc822.Message(mesg)
            name, email = msg.getaddr("From")
            emailpath = str(self._emailpath + self._inboxfolder + "\\" + email + "_" + msg.getheader("Subject") + ".eml")
            emailpath = self._replace_whitespace(emailpath)
            file = open(emailpath,"wb")
            file.write(text)
            file.close()
            self.popinstance.dele(thisNum)
    finally:
        self.logger.info(self.popinstance.quit())

def _replace_whitespace(self,name):
    name = str(name)
    return name.replace(" ", "_")

同样在 _replace_whitespace 方法中，我希望有某种清理例程，它可以删除所有可能导致处理的非法字符。

基本上我想以标准方式将电子邮件写入收件箱目录。

我在这里做错了吗？

4

3 回答 3

3

我没有看到该代码有任何重大错误——它的行为是否不正确，或者您只是在寻找一般的样式指南？

几点注意事项：

  1. 而不是logger.info ("foo %s %s" % (bar, baz))，使用"foo %s %s", bar, baz. 如果不打印消息，这可以避免字符串格式化的开销。
  2. 放一个try...finally左右开口emailpath。
  3. 使用'\n'.join (body), 而不是string.join (body, '\n').
  4. 而不是msg.getaddr("From")，只是msg.From。
于 2008-10-22T06:58:57.163 回答
1

这不是重构（据我所知，它不需要重构），而是一些建议：

您应该使用电子邮件包而不是 rfc822。将 rfc822.Message 替换为 email.Message，并使用 email.Utils.parseaddr(msg["From"]) 获取姓名和电子邮件地址，使用 msg["Subject"] 获取主题。

使用 os.path.join 创建路径。这：

emailpath = str(self._emailpath + self._inboxfolder + "\\" + email + "_" + msg.getheader("Subject") + ".eml")

变成：

emailpath = os.path.join(self._emailpath + self._inboxfolder, email + "_" + msg.getheader("Subject") + ".eml")

（如果 self._inboxfolder 以斜杠开头或 self._emailpath 以一个结尾，您也可以将第一个 + 替换为逗号）。

它并没有真正伤害任何东西，但您可能不应该使用“file”作为变量名，因为它会影响内置类型（像 pylint 或 pychecker 这样的检查器会警告您）。

如果您没有在此函数之外使用 self.popinstance（鉴于您在函数内连接并退出，似乎不太可能），那么将其作为 self 的属性是没有意义的。只需单独使用“popinstance”。

使用 xrange 代替范围。

不只是导入 StringIO，而是这样做：

try:
    import cStringIO as StringIO
except ImportError:
    import StringIO

如果这是一个可以由多个客户端一次访问的 POP 邮箱，您可能需要在 RETR 调用周围放置一个 try/except 以在您无法检索到一条消息时继续。

正如 John 所说，使用 "\n".join 而不是 string.join，使用 try/finally 仅在打开文件时关闭文件，并单独传递日志记录参数。

我能想到的一个重构问题是您实际上并不需要解析整个消息，因为您只是转储原始字节的副本，而您想要的只是 From 和 Subject 标头。您可以改为使用 popinstance.top(0) 来获取标头，从中创建消息（空白正文），并将其用于标头。然后执行完整的 RETR 以获取字节。仅当您的消息很大时才值得这样做（因此解析它们需要很长时间）。在进行此优化之前，我肯定会进行测量。

对于您清理名称的功能，这取决于您希望名称有多好，以及您对电子邮件和主题使文件名唯一性的确定程度（似乎不太可能）。您可以执行以下操作：

emailpath = "".join([c for c in emailpath if c in (string.letters + string.digits + "_ ")])

你最终只会得到字母数字字符、下划线和空格，这看起来像是一个可读的集合。鉴于您的文件系统（使用 Windows）可能不区分大小写，您也可以将其小写（将 .lower() 添加到末尾）。如果你想要更复杂的东西，你可以使用 emailpath.translate。

于 2008-10-22T09:32:26.060 回答
0

除了我对约翰回答的评论

我发现问题出在哪里，名称字段和主题字段中有非法字符，这导致 python 遇到问题，因为它在看到“：”和“/”后试图将电子邮件写为目录。

约翰第 4 点不起作用！所以我像以前一样离开了它。第 1 点也是正确的，我是否正确实施了您的建议？

def _dump_pop_emails(self):
    self.logger.info("open pop account %s with username: %s", self.account[0], self.account[1])
    self.popinstance = poplib.POP3(self.account[0])
    self.logger.info(self.popinstance.getwelcome()) 
    self.popinstance.user(self.account[1])
    self.popinstance.pass_(self.account[2])
    try:
        (numMsgs, totalSize) = self.popinstance.stat()
        for thisNum in range(1, numMsgs+1):
            (server_msg, body, octets) = self.popinstance.retr(thisNum)
            text = '\n'.join(body)
            mesg = StringIO.StringIO(text)                               
            msg = rfc822.Message(mesg)
            name, email = msg.getaddr("From")
            emailpath = str(self._emailpath + self._inboxfolder + "\\" + self._sanitize_string(email + " " + msg.getheader("Subject") + ".eml"))
            emailpath = self._replace_whitespace(emailpath)
            print emailpath
            file = open(emailpath,"wb")
            file.write(text)
            file.close()
            self.popinstance.dele(thisNum)
    finally:
        self.logger.info(self.popinstance.quit())

def _replace_whitespace(self,name):
    name = str(name)
    return name.replace(" ", "_")   

def _sanitize_string(self,name):
    illegal_chars = ":", "/", "\\"
    name = str(name)
    for item in illegal_chars:
        name = name.replace(item, "_")
    return name
于 2008-10-22T07:30:20.347 回答