HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

A simple dictionary tool, extensible with plugins

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
extensiblesimplewithpluginsdictionarytool

Problem

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 ''

@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):
mat

Solution

A quick review of the base classes.

-
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?

-
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.

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

-
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)


-
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.)

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

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

Code Snippets

entry.content['id']
def __repr__(self):
    return '{0.__name__}({1.id}, {1.name})'.format(type(self), self)

Context

StackExchange Code Review Q#59867, answer score: 7

Revisions (0)

No revisions yet.