I know that this is a complete mess and does not even come close to fitting PEP8. That's why I'm posting it here. I need help making it better.

bookName      = None
authorName    = None
bookStartLine = None
bookEndLine   = None

def New_book(object):
    def __init__(self, currentBookName, currentAuthorName, currentBookStartLine, currentBookEndLine):
        self.currentBookName      = currentBookName
        self.currentAuthorName    = currentAuthorName
        self.currentBookStartLine = currentBookStartLine
        self.currentBookEndLine   = currentBookEndLine
import urllib.request

def parser(currentUrl): #parses texts to extract their title, author, begginning line and end line
    global bookName
    global authorName
    global bookStartLine
    global bookEndLine
    global url

    url = 'http://www.gutenberg.org/cache/epub/1232/pg1232.txt' #machiaveli
    url = currentUrl    
    book = urllib.request.urlopen(url)
    lines = book.readlines()

    finalLines = [line.decode()[:-2] for line in lines]

    for line in finalLines:
        if "Title" in line:
            currentBookName = line[7:len(line)]
    for line in finalLines:
        if "Author" in line:
            currentAuthorName = line[8:len(line)]
    currentBookStartLine,currentBookEndLine = False,False

    for index,line in enumerate(line,start=1):
        if "*** START OF THE PROJECT" in line:
            currentBookStartLine = index

        if "*** END OF THE PROJECT" in line:
            currentBookEndLine = index

    url           = currentUrl
    bookName      = currentBookName
    authorName    = currentAuthorName
    bookStartLine = currentBookStartLine
    bookEndLine   = currentBookEndLine

parser('http://www.gutenberg.org/cache/epub/768/pg768.txt') #wuthering heights
  • \$\begingroup\$ I know that this is a complete mess. If you know something is a complete mess I'm sure you have a reason to think so. Focus on the why and try to change it. \$\endgroup\$ – Alex May 14 '13 at 16:02
  • No need for a New_Book class with something so simple – I recommend a named tuple instead.
  • HTTPResponse objects don't seem to have a readlines() method (in Python 3) – this forces you to explicitly split the response data with '\r\n'.
  • It's best to use the with-statement for file/resource operations. This is a CYA in case you forget to include resource.close() in your code.

Here's my refactor:

from urllib.request import urlopen
import collections

def parser(url): 
    '''parses texts to extract their title, author, begginning line and end line'''

    with urlopen(url) as res:
        lines = res.read().decode('utf-8').split('\r\n')

    title = next(line[7:] for line in lines if line.startswith("Title: "))
    author = next(line[8:] for line in lines if line.startswith("Author: "))
    start,end = False,False

    for index,line in enumerate(lines,start=1):
        if line.startswith("*** START OF THIS PROJECT"):
            start = index

        if line.startswith("*** END OF THIS PROJECT"):
            end = index - 1

    content = "".join(lines[start:end])

    return collections.namedtuple('Book', ['title', 'author', 'content', 'start', 'end'])(title, author, content, start, end)

book = parser('http://www.gutenberg.org/cache/epub/1232/pg1232.txt' ) #machiaveli <3
print("{} by {}. {} lines".format(book.title,book.author,book.end-book.start))

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.