使用windows python 2.3的传入poplib重构

Question

嗨，伙计们，你们能帮我重构一下，让它变得有意义吗？

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方法中，我希望有某种清理例程，它可以删除所有可能导致处理的非法字符。

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

我是不是做错了什么？

Answer 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而不是range。

执行以下操作，而不是只导入StringIO：

try:
    import cStringIO as StringIO
except ImportError:
    import StringIO

如果这是一个可由多个客户端同时访问的POP邮箱，您可能希望在无法检索到一条消息的情况下，在RETR呼叫周围放置try/except以继续。

正如约翰所说，使用"\n".join而不是string.join，使用try/finally只在文件打开时关闭它，并分别传递日志记录参数。

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

要让您的函数清理名称，这取决于您希望名称有多好，以及您有多确定电子邮件和主题使文件名唯一(似乎相当不可能)。你可以这样做：

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

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

Answer 2

我看不出这段代码有什么大问题--它的行为是不正确的，还是您只是在寻找通用的风格准则？

以下是一些注意事项：

1. 而不是logger.info ("foo %s %s" % (bar, baz))，请使用"foo %s %s", bar, baz。这样就避免了在不打印消息的情况下设置字符串格式的开销。
2. 在打开msg.From.

'\n'.join (body)而不是msg.getaddr("From")的string.join (body, '\n').

• Instead时放了一个try...finally

Answer 3

我对约翰的回答的进一步评论

我找出了问题所在，name字段和Subject字段中有非法字符，这导致python在看到":“和"/”后试图将电子邮件写成目录时遇到了问题。

John point 4号不起作用！所以我像以前一样把它留下了。另外，第一点是正确的吗，我是否正确地实施了您的建议？

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