2012-11-10 71 views
6

यह मेरा पहला प्रश्न है, और अगर मैं कोड-उदाहरण पक्ष पर थोड़ा सा समय माफी माँगता हूं तो मैं क्षमा चाहता हूं।पायथन - क्या इस कोड में सूची समझ और जेनरेटर की कमी है

नौकरी आवेदन के हिस्से के रूप में मुझे एक बिट टोरेंट फ़ाइल पार्सर लिखने के लिए कहा गया था जो कुछ क्षेत्रों का खुलासा करता था। मैंने कोड किया, और कहा गया कि मेरा कोड "उस स्तर पर काफी नहीं था जिसे हमें टीम लीड से जरूरी है"। आउच!

यह ठीक है, मैंने कोड किए हैं, और समझ की सूची के वर्षों से, जनरेटर दिन में वापस नहीं थे (मैंने कोबोल के साथ शुरू किया था, लेकिन सी, सी ++, आदि के साथ कोड किया है)। मेरे लिए नीचे कोड बहुत साफ है। कभी-कभी कोई अधिक जटिल संरचनाओं, वाक्यविन्यास या पैटर्न का उपयोग करने की आवश्यकता है - "इसे सरल रखें"।

क्या मैं कुछ पाइथन गुरु से इस कोड की आलोचना करने के लिए कह सकता हूं? मेरा मानना ​​है कि यह दूसरों के लिए उपयोगी है कि कोड कहां सुधार किया जा सकता है। अधिक टिप्पणी नहीं थे, आदि (bencode.py http://wiki.theory.org/Decoding_bencoded_data_with_python से है)

क्षेत्रों मैं के बारे में सोच सकते हैं

: display_ में

  1. * तरीकों सूची comprehensions उपयोग करने के लिए "अगर है" की स्ट्रिंग से बचने के लिए बेहतर
  2. सूची समझ/जनरेटर उपयोग
  3. वैश्विक
  4. stdin/stdout/पाइपिंग का बुरा उपयोग? यह एक साधारण असाइनमेंट था, इसलिए मैंने सोचा कि यह आवश्यक नहीं था।

मुझे व्यक्तिगत रूप से इस कोड पर गर्व था, इसलिए मुझे यह जानना होगा कि मुझे कहां सुधार करना है। धन्यवाद।

#!/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

मैं इसे पहले कक्षा में बदल दूंगा। – Blender

+1

यह ऑब्जेक्ट अभिविन्यास गायब है। मेरी शर्त यह है कि असली चीज़ है जिसे देखा गया था। 'If __name__ ...' के तहत भी हिस्सा बहुत लंबा है। यह एक मुख्य() समारोह होना चाहिए। – Keith

+4

पहली बात जो मैंने देखा वह वैश्विक चर है। – icktoofay

उत्तर

3

पहली बात यह है कि मुझे लगता है कि आपके पास बहुत सारे वैश्विक चर हैं। यह अच्छा नहीं है; एक कोड के लिए आपका कोड अब थ्रेडसेफ नहीं है। (मैं अब देखते हैं कि आप अपने प्रश्न में है कि बताया गया है, लेकिन यह है कि कुछ है जो परिवर्तित हो सकती है।)

यह एक छोटे से अजीब लग रहा है: इसके बजाय

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

, तो आप सिर्फ यह कर सकता है:

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

मैं यह भी ध्यान देता हूं कि आप फ़ाइल को बहुत सारे मुद्रित डेटा को आउटपुट करने के लिए पर्याप्त रूप से विश्लेषण करते हैं। आप इसे उचित संरचनाओं में रखना चाहते हैं। उदाहरण के लिए, Torrent, Piece, और File कक्षाएं हैं।

+0

धन्यवाद सब - मैं आपकी सभी टिप्पणियों से सहमत हूं। मुझे लगता है कि मैं शैली के विरोध के रूप में परिणाम पर अधिक ध्यान केंद्रित किया गया था। पर्याप्त मेला और फिर से धन्यवाद। –

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 लूप में स्पष्ट इंडेक्स वैरिएबल मैनिपुलेशन की आवश्यकता नहीं होनी चाहिए।

+0

भी सहमत हैं। यह निश्चित रूप से पायथन सीखने में मेरा कमजोर बिंदु है। अनुभव के साथ जानें ... –

 संबंधित मुद्दे

  • कोई संबंधित समस्या नहीं^_^