CODEWITHSUNDEEP
pythonoopclassmethods
tijko
tijko

Reputation: 8292

Using python class methods

I have a script that I had originally left as one long function.

#! /usr/bin/env python
import mechanize
from BeautifulSoup import BeautifulSoup
import sys
import sqlite3

def dictionary(word):
    br = mechanize.Browser()
    response = br.open('http://www.dictionary.reference.com')
    br.select_form(nr=0)
    br.form['q'] = word 
    br.submit()
    definition = BeautifulSoup(br.response().read())
    trans = definition.findAll('td',{'class':'td3n2'})
    fin = [i.text for i in trans]
    query = {}
    word_count = 1
    def_count = 1
    for i in fin: 
        query[fin.index(i)] = i
    con = sqlite3.connect('/home/oberon/vocab_database/vocab.db')
    with con:
        spot = con.cursor()
        spot.execute("SELECT * FROM Words")
        rows = spot.fetchall()
        for row in rows:
            word_count += 1
        spot.execute("INSERT INTO Words VALUES(?,?)", (word_count,word))
        spot.execute("SELECT * FROM Definitions")
        rows = spot.fetchall()
        for row in rows:
            def_count += 1
        for q in query:
            spot.execute("INSERT INTO Definitions VALUES(?,?,?)", (def_count,query[q],word_count))
            def_count += 1
    return query

print dictionary(sys.argv[1])

Now, I wanted to practice OOP form by creating a class. I thought it would be best to separate this into at least a couple of functions too.

I came up with:

#! /usr/bin/env python
import mechanize
from BeautifulSoup import BeautifulSoup
import sys
import sqlite3


class Vocab:
    def __init__(self):
        self.word_count = 1
        self.word = sys.argv[1] 
        self.def_count = 1
        self.query = {}

    def dictionary(self,word):
        self.br = mechanize.Browser()
        self.response = self.br.open('http://www.dictionary.reference.com')
        self.br.select_form(nr=0)
        self.br.form['q'] = word 
        self.br.submit()
        self.definition = BeautifulSoup(self.br.response().read())
        self.trans = self.definition.findAll('td',{'class':'td3n2'})
        self.fin = [i.text for i in self.trans]
        for i in self.fin: 
            self.query[self.fin.index(i)] = i
        return self.query

    def word_database(self):
        self.con = sqlite3.connect('/home/oberon/vocab_database/vocab.db')
        with self.con:
            self.spot = self.con.cursor()
            self.spot.execute("SELECT * FROM Words")
            self.rows = self.spot.fetchall()
            for row in self.rows:
                self.word_count += 1
            self.spot.execute("INSERT INTO Words VALUES(?,?)", (self.word_count,self.word))
            self.spot.execute("SELECT * FROM Definitions")
            self.rows = self.spot.fetchall()
            for row in self.rows:
                self.def_count += 1
            for q in self.query:
                self.spot.execute("INSERT INTO Definitions VALUES(?,?,?)", (self.def_count,self.query[q],self.word_count))
                self.def_count += 1



Vocab().dictionary(sys.argv[1])

I know that on the last line when I call Vocab().dictionary(sys.argv[1]) this is only going to run the dictionary method. I am trying to figure out how to invoke the word_database method everytime I run the script still.

Is this the wrong way to go about this? Should I of just left these methods as just one large method?

Upvotes: 0

Views: 211

Answers (5)

Brenden Brown
Brenden Brown

Reputation: 3215

I'd recommend first refactoring without going object-oriented. You don't gain anything by making a class. Just define your two methods, and at the end:

dictionary(sys.argv[1)
word_database()

Upvotes: 0

Pyrce
Pyrce

Reputation: 8571

A couple of things:

First, you don't need to make all variables self.var_name just because you're wrapping them in a class. If the variable isn't needed after the function call finishes, just use local variables.

Second, to get word_database to have been called each time Vocab uses dictionary adding self.word_database() to your init function __init__(self, word). This will ensure those features are always available.

Third, if you're just going to treat the object as a script and do Vocab().dictionary(word) you're probably better off with not using a class structure. If you plan on computing some of the work with Vocab() and then doing other work incrementally (calling dictionary repeatedly) then keep the class structure. But the way you're using it currently is like a function call. (If you keep function call semantics you should at least break that original function into smaller parts).

Upvotes: 1

Michael
Michael

Reputation: 2261

You just need to call word_database at the same point you would have in the original script.

def dictionary(self,word):
    self.br = mechanize.Browser()
    self.response = self.br.open('http://www.dictionary.reference.com')
    self.br.select_form(nr=0)
    self.br.form['q'] = word 
    self.br.submit()
    self.definition = BeautifulSoup(self.br.response().read())
    self.trans = self.definition.findAll('td',{'class':'td3n2'})
    self.fin = [i.text for i in self.trans]
    for i in self.fin: 
        self.query[self.fin.index(i)] = i

    # Continue the script...
    self.word_database()

    return self.query

Upvotes: 1

kindall
kindall

Reputation: 184191

I'm not really sure there is any advantage to it being a class, but if you want to call multiple methods of an instance, you will need to assign a name to the instance.

vocab = Vocab()
vocab.dictionary(...)
vocab.word_database(...)

Upvotes: 1

Antimony
Antimony

Reputation: 39451

It looks fine as a script to me. The only real question is why you do 

for row in rows:
        word_count += 1

instead of 

word_count += len(rows)

As for your question, you call word_database by doing

self.word_database()

Upvotes: 1

Related Questions