patternpythonMinor
A simple dictionary tool, extensible with plugins
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
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
which seems unnecessarily verbose compared to something like
-
In
-
Good practice for
-
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
-
Why are
-
What is the purpose of the
-
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.