6

これは私の最初の質問であり、コード例側で少し長い場合は申し訳ありません。

求人応募の一環として、いくつかのフィールドを公開する Bit Torrent ファイル パーサーを作成するように依頼されました。私がコードを書いたところ、私のコードは「チーム リーダーから要求されるレベルに達していない」と言われました。痛い!

それは結構です、私がコーディングしてから何年も経ち、内包表記、ジェネレーターは当時存在しませんでした(私はCOBOLで始めましたが、C、C ++などでコーディングしました)。私にとって、以下のコードは非常にきれいです。より複雑な構造、構文、またはパターンを使用する必要がない場合もあります-「シンプルに保つ」。

Python の第一人者にこのコードの批評をお願いできますか? コードを改善できる場所を確認することは、他の人にとって役立つと思います。さらにコメントなどがありました (bencode.py はhttp://wiki.theory.org/Decoding_bencoded_data_with_pythonからのものです) 。

私が考えることができる分野:

  1. display_* メソッドでリスト内包表記を使用して、「if's」の文字列を回避する
  2. リスト内包表記 / ジェネレーターの使用法
  3. グローバルの悪用
  4. 標準入力/標準出力/配管? これは簡単な課題だったので、必要ないと思いました。

私は個人的にこのコードを誇りに思っていたので、どこを改善する必要があるか知りたい. ありがとう。

    #!/usr/bin/env python2
    """Bit Torrent Parsing

    Parses a Bit Torrent file.


    A basic parser for Bit Torrent files.  Visit http://wiki.theory.org/BitTorrentSpecification for the BitTorrent specification.

    """

    __author__ = "...."
    __version__ = "$Revision: 1.0 $"
    __date__ = "$Date: 2012/10/26 11:08:46 $"
    __copyright__ = "Enjoy & Distribute"
    __license__ = "Python"

    import bencode
    import argparse
    from argparse import RawTextHelpFormatter
    import binascii
    import time
    import os
    import pprint

    torrent_files = 0
    torrent_pieces = 0

    def display_root(filename, root):
        """prints main (root) information on torrent"""
        global torrent_files
        global torrent_pieces

        print
        print "Bit Torrent Metafile Structure root nodes:"
        print "------------------------------------------"
        print "Torrent filename: ", filename
        print "    Info:           %d file(s), %d pieces, ~%d kb/pc" % (
                   torrent_files, 
                   torrent_pieces, 
                   root['info']['piece length'] / 1024)

        if 'private' in root['info']:
            if root['info']['private'] == 1:
                print "                      Publish presence: Private"

        print "    Announce:      ", root['announce']

        if 'announce-list' in root:
            print "    Announce List: "
            for i in root['announce-list']:
                print "                   ", i[0]  

        if 'creation date' in root:
            print "    Creation Date: ", time.ctime(root['creation date'])

        if 'comment' in root:
            print "    Comment:       ", root['comment']

        if 'created-by' in root:
            print "    Created-By:    ", root['created-by']

        print "    Encoding:      ", root['encoding']
        print



    def display_torrent_file(info):
        """prints file information (single or multifile)"""
        global torrent_files
        global torrent_pieces

        if 'files' in info:
            # multipart file mode
            # directory, followed by filenames

            print "Files:"

            max_files = args.maxfiles 
            display = max_files if (max_files < torrent_files) else torrent_files
            print "    %d File %d shown: " % (torrent_files, display)
            print "    Directory: ", info['name']
            print "    Filenames:"

            i = 0
            for files in info['files']:

                if i < max_files:

                    prefix = ''
                    if len(files['path']) > 1:
                        prefix = './'
                    filename = prefix + '/'.join(files['path'])

                    if args.filehash: 
                        if 'md5sum' in files:
                            md5hash = binascii.hexlify(files['md5sum'])
                        else:
                            md5hash = 'n/a'
                        print '        %s [hash: %s]' % (filename, md5hash)
                    else:
                        print '        %s ' % filename
                    i += 1
                else:
                    break

        else:
            # single file mode
            print "Filename: ", info['name']

        print


    def display_pieces(pieceDict):
        """prints SHA1 hash for pieces, limited by arg pieces"""
        global torrent_files
        global torrent_pieces 
      #  global pieceDict  

        # limit since a torrent file can have 1,000's of pieces
        max_pieces = args.pieces if args.pieces else 10

        print "Pieces:" 
        print "    Torrent contains %s pieces, %d shown."% (
              torrent_pieces, max_pieces)

        print "    piece : sha1"
        i = 0           
        while i < max_pieces and i < torrent_pieces:
            # print SHA1 hash in readable hex format
            print '    %5d : %s' % (i+1, binascii.hexlify(pieceDict[i]))
            i += 1


    def parse_pieces(root):
        """create dictionary [ piece-num, hash ] from info's pieces  

        Returns the pieces dictionary.  key is the piece number, value is the
        SHA1 hash value (20-bytes) 

        Keyword arguments:
        root -- a Bit Torrent Metafile root dictionary

        """

        global torrent_pieces 

        pieceDict = {}
        i = 0
        while i < torrent_pieces:
            pieceDict[i] = root['info']['pieces'][(20*i):(20*i)+20]
            i += 1   

        return pieceDict

    def parse_root_str(root_str):
        """create dictionary [ piece-num, hash ] from info's pieces  

        Returns the complete Bit Torrent Metafile Structure dictionary with 
        relevant Bit Torrent Metafile nodes and their values.

        Keyword arguments:
        root_str -- a UTF-8 encoded string with root-level nodes (e.g., info)

        """

        global torrent_files
        global torrent_pieces


        try:
            torrent_root = bencode.decode(root_str)
        except StandardError:
            print 'Error in torrent file, likely missing separators like ":"'

        if 'files' in torrent_root['info']:
            torrent_files = len(torrent_root['info']['files'])
        else:
            torrent_files = 1

        torrent_pieces = len(torrent_root['info']['pieces']) / 20
        torrent_piece = parse_pieces(torrent_root)

        return torrent_root, torrent_piece   

    def readfile(filename):
        """read  file and return file's data"""

        global torrent_files
        global torrent_pieces

        if os.path.exists(filename):  
            with open(filename, mode='rb') as f:
               filedata = f.read()
        else:
            print "Error: filename: '%s' does not exist." % filename
            raise IOError("Filename not found.")

        return filedata


    if __name__ == "__main__":

        parser = argparse.ArgumentParser(formatter_class=RawTextHelpFormatter,
            description=
                  "A basic parser for Bit Torrent files.  Visit "
                  "http://wiki.theory.org/BitTorrentSpecification for "
                  "the BitTorrent specification.",
            epilog=
                  "The keys for the Bit Torrent MetaInfo File Structure "
                  "are info, announce, announce-list, creation date, comment, "
                  "created by and encoding. \n"
                  "The Info Dictionary (info) is dependant on whether the torrent "
                  "file is a single or multiple file.  The keys common to both "
                  "are piece length, pieces and private.\nFor single files, the "
                  "additional keys are name, length and md5sum.For multiple files " 
                  "the keys are, name and files.  files is also a dictionary with " 
                  "keys length, md5sum and path.\n\n"
                  "Examples:\n"
                  "torrentparse.py --string 'l4:dir14:dir28:file.exte'\n"
                  "torrentparse.py --filename foo.torrent\n"
                  "torrentparse.py -f foo.torrent -f bar.torrent "
                  "--maxfiles 2 --filehash  --pieces 2  -v")           
        filegroup = parser.add_argument_group('Input File or String')
        filegroup.add_argument("-f", "--filename", 
                   help="name of torrent file to parse",
                   action='append')
        filegroup.add_argument("-fh", "--filehash", 
                   help="display file's MD5 hash",
                   action = "store_true") 
        filegroup.add_argument("-maxf", "--maxfiles",
                   help="display X filenames (default=20)",
                   metavar = 'X',
                   type=int, default=20) 

        piecegroup = parser.add_argument_group('Torrent Pieces')
        piecegroup.add_argument("-p", "--pieces", 
                   help = "display X piece's SHA1 hash (default=10)", 
                   metavar = 'X',
                   type = int)

        parser.add_argument("-s", "--string",
                   help="string for bencoded dictionary item")


        parser.add_argument("-v", "--verbose", 
                   help = "Display MetaInfo file to stdout",
                   action = "store_true")

        args = parser.parse_args()



        if args.string:
            print 
            text = bencode.decode(args.string)
            print text
        else:    
            for fn in args.filename:

                try:
                    filedata = readfile(fn)
                    torrent_root, torrent_piece = parse_root_str(filedata)
                except IOError: 
                    print "Please enter a valid filename"
                    raise

                if torrent_root:
                    display_root(fn, torrent_root)
                    display_torrent_file(torrent_root['info'])

                    if args.pieces:
                        display_pieces(torrent_piece)

                verbose = True if args.verbose else False
                if verbose:
                    print
                    print "Verbose Mode: \nPrinting root and info dictionaries"
                    # remove pieces as its long. display it afterwards
                    pieceless_root = torrent_root
                    del pieceless_root['info']['pieces']
                    pp = pprint.PrettyPrinter(indent=4)
                    pp.pprint(pieceless_root)
                    print

                    print "Print info's piece information: "
                    pp.pprint(torrent_piece)
                    print
                print "\n"
4

2 に答える 2

4

次のスニペット:

i = 0
while i < torrent_pieces:
    pieceDict[i] = root['info']['pieces'][(20*i):(20*i)+20]
    i += 1

次のものに置き換える必要があります。

for i in range(torrent_pieces):
    pieceDict[i] = root['info']['pieces'][(20*i):(20*i)+20]

それは彼らが見たいものかもしれません。for一般に、Python コードでは、ループ内での明示的なインデックス変数の操作はあまり必要ありません。

于 2012-11-10T03:22:20.313 に答える
3

最初に気付くのは、大量のグローバル変数があることです。それは良くないね; 1 つの問題として、コードはもはやスレッドセーフではありません。(あなたの質問でそれを指摘したことがわかりましたが、それは変更すべきものです。)

これは少し奇妙に見えます:

i = 0
for files in info['files']:
    if i < max_files:
        # ...
    else:
        break

代わりに、これを行うことができます:

for file in info['files'][:max_files]:
    # ...

また、すべてのデータをきれいに出力するのに十分なだけファイルを解析していることにも気付きました。それを適切な構造に入れたいと思うかもしれません。たとえば、TorrentPiece、およびFileクラスがあります。

于 2012-11-10T03:21:34.173 に答える