Skip to content
This repository has been archived by the owner on Jan 10, 2023. It is now read-only.

Commit

Permalink
Add empty separator check and tests
Browse files Browse the repository at this point in the history
str.split method does not accept an empty string as a separator:

	>>> 'foo'.split('')
	Traceback (most recent call last):
	  File "<stdin>", line 1, in <module>
	ValueError: empty separator
	>>>

Add an explicit test in the StringTrie constructor so we fail early
when constructing the object.

(As an aside, str.split allows None as a separator.  StringTrie could
allow None separator but _key_from_path would have to be changed.  If
anyone needs that feature it’s fairly easy to implement).

[mina86@mina86.com: expanded commit message]
  • Loading branch information
Waren Long authored and mina86 committed Aug 14, 2017
1 parent 0e1a9b8 commit 64ee083
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 1 deletion.
12 changes: 11 additions & 1 deletion pygtrie.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ def _sorted_iteritems(d):
_iteritems = lambda d: iter(d.items()) # pylint: disable=invalid-name
_iterkeys = lambda d: iter(d.keys()) # pylint: disable=invalid-name

try:
_basestring = basestring
except NameError:
_basestring = str


class ShortKeyError(KeyError):
"""Raised when given key is a prefix of a longer key."""
Expand Down Expand Up @@ -1220,7 +1225,12 @@ def __init__(self, *args, **kwargs):
named argument is not specified on the function's prototype
because of Python's limitations.
"""
self._separator = kwargs.pop('separator', '/')
separator = kwargs.pop('separator', '/')
if not isinstance(separator, _basestring):
raise TypeError('separator must be a string')
if not separator:
raise ValueError('separator can not be empty')
self._separator = separator
super(StringTrie, self).__init__(*args, **kwargs)

@classmethod
Expand Down
13 changes: 13 additions & 0 deletions test.py
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,19 @@ def path_from_key(cls, key):
def key_from_path(cls, path):
return '/'.join(path)

def test_valid_separator(self):
t = pygtrie.StringTrie()
t['foo/bar'] = 42
self.assertTrue(bool(t.has_node('foo') & pygtrie.Trie.HAS_SUBTRIE))

t = pygtrie.StringTrie(separator='.')
t['foo.bar'] = 42
self.assertTrue(bool(t.has_node('foo') & pygtrie.Trie.HAS_SUBTRIE))

def test_invalid_separator(self):
self.assertRaises(TypeError, pygtrie.StringTrie, separator=42)
self.assertRaises(ValueError, pygtrie.StringTrie, separator='')


class SortTest(unittest.TestCase):

Expand Down

0 comments on commit 64ee083

Please sign in to comment.