1

こんにちはみんな、これをリファクタリングして、賢明に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 メソッドでは、処理を引き起こす可能性のあるすべての不正な文字を取り除く何らかのクリーニング ルーチンが必要です。

基本的に、標準的な方法でメールを受信トレイ ディレクトリに書き込みたいと考えています。

私はここで何か間違っていますか？

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 ではなく、email パッケージを使用する必要があります。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」を単独で使用してください。

range の代わりに xrange を使用します。

StringIO をインポートする代わりに、次のようにします。

try:
    import cStringIO as StringIO
except ImportError:
    import StringIO

これが一度に複数のクライアントからアクセスできる POP メールボックスである場合は、RETR 呼び出しの前後に try/except を配置して、1 つのメッセージを取得できない場合に続行することをお勧めします。

ジョンが言ったように、string.join ではなく "\n".join を使用し、try/finally を使用してファイルが開いている場合にのみファイルを閉じ、ログ パラメータを個別に渡します。

私が考えることができる 1 つのリファクタリングの問題は、生のバイトのコピーをダンプしているだけであり、必要なのは 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 に答える