传入poplib重构使用Windows python 2.3

Question

嗨，大家好请你帮我重构这个，所以它是明智pythonic。

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。使用email.Message替换rfc822.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一个结束，你可以用逗号同时更换第一+）。它不会真的伤害任何东西，但你可能不应该使用“文件”作为变量名称，因为它会隐藏内置类型（像pylint或pychecker这样的检查器会警告你）。

如果你没有在这个函数之外使用self.popinstance（似乎不太可能，因为你在函数中连接和退出），那么没有必要将它作为self的一个属性。只需使用“popinstance”本身。

使用xrange代替范围。

而不只是进口StringIO的，这样做：

try: 
    import cStringIO as StringIO 
except ImportError: 
    import StringIO 

如果这是一个可以由多个客户端同时访问的POP邮箱，你可能想要把一个try /唯独身边的如果您无法检索一条消息，RETR呼叫将继续。

正如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。

Answer 2

我没有看到该代码的任何重大错误 - 它的行为不正确，或者你只是在寻找一般的风格指南？

的几个注意事项：

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')。代替msg.getaddr("From")，只是msg.From。

Answer 3

继我对约翰的回答评论

我发现了什么问题是，有在名称字段和标题栏，导致蟒蛇得到打嗝非法字符，因为它试着写看到“：”和“/”后，将电子邮件作为目录。

约翰点数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