8
\$\begingroup\$

I'm working on a simple dictionary tool, with a base class that can be extended by plugins to represent different dictionaries. The base class does most of the heavy lifting: it keeps the index of all entries in memory, and it handles searching the index. Plugins that extend this class implement populating the index and loading the entries on demand, handling the specifics of the dictionary backend, such as the formatting of entries.

These are the base classes:

import abc
from collections import defaultdict


class BaseEntry(object):
    def __init__(self, entry_id, name):
        self.entry_id = entry_id
        self.name = name

    @property
    def content(self):
        return {
            'id': self.entry_id,
            'name': self.name,
            'content': [],
            'references': [],
        }

    def __repr__(self):
        return '%s: %s' % (self.entry_id, self.name)


class BaseDictionary(object):
    @abc.abstractproperty
    def name(self):
        return '<The Dictionary>'

    @abc.abstractproperty
    def is_public(self):
        return False

    @property
    def license(self):
        return None

    def __init__(self):
        self.items_sorted = {}
        self.items_by_name = defaultdict(list)
        self.items_by_id = {}
        self.load_index()

    def find(self, word, find_similar=False):
        matches = self.items_by_name.get(word)
        if matches:
            return matches
        if find_similar:
            return self.find_by_prefix(word, find_similar=True)
        return []

    def find_by_prefix(self, prefix, find_similar=False):
        matches = []
        for k in self.items_sorted:
            if k.startswith(prefix):
                matches.extend(self.items_by_name[k])
            elif matches:
                break
        if find_similar and not matches and len(prefix) > 1:
            return self.find_by_prefix(prefix[:-1], find_similar=True)
        return matches

    def find_by_suffix(self, suffix):
        matches = []
        for k in self.items_sorted:
            if k.endswith(suffix):
                matches.extend(self.items_by_name[k])
        return matches

    def find_by_partial(self, partial):
        matches = []
        for k in self.items_sorted:
            if partial in k:
                matches.extend(self.items_by_name[k])
        return matches

    def get_entry(self, entry_id):
        entry = self.items_by_id.get(entry_id)
        if entry:
            return [entry]
        else:
            return []

    def add(self, entry):
        self.items_by_name[entry.name].append(entry)
        self.items_by_id[entry.entry_id] = entry

    def reindex(self):
        self.items_sorted = sorted(self.items_by_name)

    @abc.abstractmethod
    def load_index(self):
        """
        Populate the index. Implement like this:
            for entry in entries:
                self.add(entry)
            self.reindex()
        :return:
        """
        pass

This is an example plugin implementation:

import os
import re

from settings import dictionary_path
from dictionary.base import BaseDictionary, BaseEntry, lazy_property

INDEX_PATH = os.path.join(dictionary_path, 'index.dat')

re_strong_defs = re.compile(r'(Defn:|Syn\.)')
re_strong_numdots = re.compile(r'(\d+\. )')
re_strong_alphadots = re.compile(r'(\([a-z]\))')
re_em_roundbr = re.compile(r'(\([A-Z][a-z]+\.\))')
re_em_squarebr = re.compile(r'(\[[A-Z][a-z]+\.\])')


def load_entry_content(word, filename):
    path = os.path.join(dictionary_path, filename)
    if not os.path.isfile(path):
        return
    with open(path) as fh:
        count = 0
        content = []
        definition_list = []
        for line in fh:
            # first line contains the term, and ignore next 2 lines
            if count < 3:
                if count == 0:
                    word = line.strip().lower()
                count += 1
                continue
            line = line.strip()
            line = line.replace('*', '')
            line = re_strong_defs.sub(r'**\1**', line)
            line = re_strong_numdots.sub(r'**\1** ', line)
            line = re_strong_alphadots.sub(r'**\1**', line)
            line = re_em_roundbr.sub(r'*\1*', line)
            line = re_em_squarebr.sub(r'*\1*', line)

            if line:
                content.append(line)
            else:
                definition_list.append(['', ' '.join(content)])
                content = []
        return {
            'id': filename,
            'name': word,
            'content': definition_list,
            'references': []
        }    


class Dictionary(BaseDictionary):
    @property
    def name(self):
        return 'Webster\'s Unabridged Dictionary'

    @property
    def is_public(self):
        return True

    @property
    def license(self):
        return """
        The content of this dictionary is for the use of anyone anywhere
        at no cost and with almost no restrictions whatsoever.
        You may copy it, give it away or re-use it under the terms of
        the Project Gutenberg License included online at www.gutenberg.net"""

    def load_index(self):
        with open(INDEX_PATH) as fh:
            for line in fh:
                (entry_id, name) = line.strip().split(':')
                entry = Entry(entry_id, name)
                self.add(entry)
        self.reindex()

    def get_entry(self, entry_id):
        entries = super(Dictionary, self).get_entry(entry_id)
        if not entries:
            entry = Entry(entry_id, '')
            if entry.content:
                entry.name = entry.content['name']
                self.add(entry)
                return [entry]
        return entries


class Entry(BaseEntry):
    @lazy_property
    def content(self):
        return load_entry_content(self.name, self.entry_id)

An example dictionary file looks like this:

chair
Chair, n. Etym: [OE. chaiere, chaere, OF. chaiere, chaere, F. chaire]

1. A movable single seat with a back.

2. An official seat, as of a chief magistrate or a judge, but esp.
that of a professor; hence, the office itself.
The chair of a philosophical school. Whewell.
A chair of philology. M. Arnold.

3. The presiding officer of an assembly; a chairman; as, to address
the chair.

4. A vehicle for one person; either a sedan borne upon poles, or two-
wheeled carriage, drawn by one horse; a gig. Shak.
Think what an equipage thou hast in air, And view with scorn two
pages and a chair. Pope.

5. An iron blok used on railways to support the rails and secure them
to the sleepers. Chair days, days of repose and age.
-- To put into the chair, to elect as president, or as chairman of a
meeting. Macaulay.
-- To take the chair, to assume the position of president, or of
chairman of a meeting.

I'm looking for a general review:

  • Is this code Pythonic?
  • Is this is good object oriented design? Would you design the class structure differently?
  • Other things you'd do differently? (Apart from using a database to handle the indexing of entries, a feature I plan to add soon.)

The open-source project is here.

\$\endgroup\$
7
\$\begingroup\$

A quick review of the base classes.

  1. There's no documentation. What does this code do? How am I supposed use it? What is the interface? When I subclass one of your base classes, what are my responsibilities? What properties and methods do I need to implement and what must they return?

  2. The interface seems inconvenient. If you want to know an entry's id, then it looks like you have to write:

    entry.content['id']
    

    which seems unnecessarily verbose compared to something like entry.id.

  3. In BaseEntry.content you construct a new dictionary each time the method is called. This seems wasteful since the dictionary is always the same.

  4. Good practice for __repr__ methods is to output something that will evaluate to an equivalent object. So I'd write:

    def __repr__(self):
        return '{0.__name__}({1.id}, {1.name})'.format(type(self), self)
    
  5. When you have an interface that needs to read the contents of a file, it's best practice to design the interface so that you can pass either a file name or a file object.

    The reason for this is that if an interface only accepts a file name, then you can only pass it data via the local file system, and that when the data comes from a network connection, or from a test case in Python source code, or is constructed in memory, then you have to save that data out to a temporary file. It is much more convenient to construct and pass a file object in these cases.

    (See for example the standard library functions tarfile.open, lzma.open, plistlib.readPlist.)

  6. Why are BaseDictionary.name and BaseDictionary.is_public abstract properties? Why do you require subclasses to override these properties?

  7. What is the purpose of the is_public property? It doesn't seem to be used.

\$\endgroup\$
  • \$\begingroup\$ Thanks a lot Gareth! To respond to some of your remarks: 1. Yup I'll add the doc; 2. entry.entry_id; 3. BaseEntry.content is only used by unit tests, implementations use a @lazy_property; 4-5. Nice idea, thanks! 6. .name and .is_public is only meaningful in implementations; 7. .is_public is used on the web interface to determine whether the dictionary should be listed or available but somewhat hidden \$\endgroup\$ – janos Sep 2 '14 at 21:42

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.