Gfrktyl

I wrote this small program to open a .torrent file, retrieve its info-hash and size. I'm still a beginner at Python so the main focus of this program was to try and utilize classes and objects instead of having just a bunch of functions. It works but I wanted to know if this is a good design. Also, I feel some of the code I wrote is redundant especially the number of times self is used.

import hashlib, bencode



class Torrent(object):



    def __init__(self, torrentfile):

        self.metainfo = bencode.bdecode(torrentfile.read())

        self.info = self.metainfo['info']

        self.files = self.metainfo['info']['files']

        self.md5hash = self.md5hash(self.info)

        self.size = self.size(self.files)



    def md5hash(self, info):

        return hashlib.sha1(bencode.bencode(info)).hexdigest()



    def size(self, files):

        filesize = 0

        for file in files:

            filesize += file['length']          

        return filesize





torrentfile = Torrent(open("test.torrent", "rb"))

print(torrentfile.md5hash)

print(torrentfile.size)

Using classes for this seems like a good idea, since you end up with a number of class instances, where each one represents a specific torrent.

In terms of the specific code, you're doing things slightly wrong in 2 ways.

Firstly, you don't need to pass instance parameters into the methods of a class. So you can access info and file as self.info and self.file, so your methods only need the self argument.

Secondly, I can see that you're doing this to try to cache the results of the method calls by overriding the methods in __init__, and while caching is good, this is a bad way of trying to achieve it.

There are 2 alternatives that spring to mind, depending on what you want to do:

If you always want the size and hash calculated when the class is instantiated, then do something similar to what you're doing now, but use different names for the data variables and the methods:

def __init__(self, torrentfile):

    self.metainfo = bencode.bdecode(torrentfile.read())

    self.info = self.metainfo['info']

    self.files = self.metainfo['info']['files']

    self.md5hash = self.calculate_md5hash()

    self.size = self.calculate_size()



def calculate_md5hash(self):

    return hashlib.sha1(bencode.bencode(self.info)).hexdigest()



def calculate_size(self):

    filesize = 0

    for file in self.files:

        filesize += file['length']          

    return filesize

Alternatively, if you only want the hash and size calculated when the methods are specifically called, but you also want caching, use lru_cache

lru_cache will cache the result of a function the first time it is run, and then simply return the result for future calls, providing the arguments to the function remain the same.

from functools import lru_cache



class Torrent(object):



    def __init__(self, torrentfile):

        self.metainfo = bencode.bdecode(torrentfile.read())

        self.info = self.metainfo['info']

        self.files = self.metainfo['info']['files']



    @lru_cache()

    def md5hash(self):

        return hashlib.sha1(bencode.bencode(self.info)).hexdigest()



    @lru_cache()

    def size(self):

        filesize = 0

        for file in self.files:

            filesize += file['length']          

        return filesize

Then call the methods explicitly:

print(torrentfile.md5hash())

print(torrentfile.size())

@MathiasEttinger picked up most of the issues. Here's a random assortment of others:

Import what you need

If you do:

from hashlib import sha1

from bencode import bencode, bdecode

Then your usage can be shortened to:

self.metainfo = bdecode(torrentfile.read())

# ...

return sha1(bencode(info)).hexdigest()

Use list comprehensions

This:

    filesize = 0

    for file in files:

        filesize += file['length']          

    return filesize

can be

return sum(f['length'] for f in files)

Use context management

You don't close your file, which is an issue; but you don't need to do it explicitly - do it implicitly:

with open("test.torrent", "rb") as torrentfile:

    torrent = Torrent(torrentfile)

print(torrent.md5hash)

print(torrent.size)

Note that this assumes Torrent is done with the file at the end of the constructor.

Use a main function

Put your global code into a main function to clean up the global namespace and allow others to use your code as a library rather than a command.