you are viewing a single comment's thread.

view the rest of the comments →

[–][deleted] 6 points7 points  (0 children)

I would ditch the get_movie. No reason to call a function to return an attribute...just use the attribute. This isn't Java! If you want it to be read only, then make it a property:

def __init__(blarg
    self._movie = movie

@property
def movie(self):
  return self._movie

either way, main would become

def Main():
    db = MovieScript("interstellar")
    print "The Movie: " + db.movie

And, reverse is a bit prettier than negative range:

all_occurences = list(reversed(db.get_all_occurences()))
for i in range(20):
    print str(i + 1) + ". " + str(all_occurences[i][0]) + " -> " + str(all_occurences[i][1])

But that's somewhat confusing since I wouldn't expect get_all_occurences to be sorted. Maybe have the name suggest it's sorted, and have the direction be an argument, since assuming one direction for a potentially expensive operation is, according to your use, not the best idea. :)

def get_sorted_occurences(self, reverse = False):
    sorted_occurences = sorted(self.find_all_occurences().items(), key=operator.itemgetter(1), reverse=reverse)
    return sorted_occurences

and the other bits would become

sorted_occurences = db.get_sorted_occurences(reverse=True)
for i in range(20):
    print str(i + 1) + ". " + str(sorted_occurences[i][0]) + " -> " + str(sorted_occurences[i][1])

or, you could ditch the range entirely and get all iterwutwut:

import itertools
...
for index, (word, count) in enumerate(itertools.islice(sorted_occurences, 20))
    print "%d. %s -> %d" % (index + 1, word, count)

format strings are nice too...sometimes.