For in-depth review of the code:
If I were to make _get_item then I'd look at the get function first.
I don't think that you should have pop in that function, and so I'll remove it.
This leaves you with:
def get(key, table):
key_hash = hashlib.sha1(str(key)).hexdigest().upper()
entry = table
for i, char in enumerate(key_hash):
ordinal = ord(char) - 48
entry = entry[ordinal]
entry_len = len(entry)
if entry_len is 0:
raise KeyError(key)
elif entry_len is 2:
return entry[1]
raise KeyError(key)
From this we can see the basic looping structure. I'd first merge ordinal with entry, to get entry = entry[ord(char) - 48].
You shouldn't use 1 is 1 instead use 1 == 1, it makes more sense as 1 == 1.0 is true where 1 is 1.0 is not.
As we need to get the item, entry, even if it's not there (we'll be using this for update too), we should change the ifs to just return the entry.
Finally you should notice that entry_len == 0 is the same as not entry.
Merging these all together gets us:
def _get_item(key, table):
key_hash = hashlib.sha1(str(key)).hexdigest().upper()
entry = table
for i, char in enumerate(key_hash):
entry = entry[ord(char) - 48]
if not entry:
return entry
elif len(entry) == 2:
return entry
else:
raise KeyError(key)
To use it is simple:
def get(key, table):
item = _get_item(key)
if item and item[0] == key:
return item[1]
raise KeyError(key)
We still need to get _get_item working for update however.
You should be able to see that the code is roughly the same for all except elif entry_len is 2:.
To help with this we should break out of the loop so that we're in a 'separate environment'.
To make things simpler I'd also return if key == entry[0].
And so:
elif len(entry) == 2:
if key == entry[0]:
return entry
break
You should notice that we don't want the new code to run if we are using the other version of this function.
To do this we can pass a flag that returns the entry after exiting the for.
if extend:
return entry
Finally we're left with:
extant_key, extant_values = entry[0], entry[1]
del entry[:]
#entry = []
extant_hash = get_hash(extant_key)
for i in xrange(i + 1, len(key_hash)):
char, extant_char = key_hash[i], extant_hash[i]
if char == extant_char:
entry.extend(alphabet())
ordinal = ord(char) - 48
entry = entry[ordinal]
#entry = []
else: break
if char != extant_char:
entry.extend(alphabet())
char_ord = ord(char) - 48
entry[char_ord].extend((key, [value]))
extant_ord = ord(extant_char) - 48
entry[extant_ord].extend((extant_key, extant_values))
#entry has empty lists except 2
break
else: pass
#hash collision, to be continued
This leads to the following changes:
Using both extant_key and extant_value is more verbose than I like, and would just use extant with the slice item[0:2].
Use izip rather than xrange(i + 1, len(key_hash)).
This is quite simple add from itertools import izip at the top, and then use:
for char, extant_char in izip(key_hash[i + 1:], extant_hash[i + 1:]):
Use item[ord(...) - 48] rather than setting intermarry variables.
Raise an error if you don't break out the for loop.
Change the if to char != extant_char and then break.
This adds a little bit of readability, and prevents large amounts of indents.
Finally return entry[ord(char) - 48].
This should lead to something like:
extant = entry[0:2]
del entry[:]
extant_hash = get_hash(extant[0])
for char, extant_char in izip(key_hash[i + 1:], extant_hash[i + 1:]):
if char != extant_char:
break
entry.extend(alphabet())
entry = entry[ord(char) - 48]
else:
raise KeyError(key)
entry.extend(alphabet())
entry[ord(extant_char) - 48].extend(extant)
return entry[ord(char) - 48]
Finally I should note that the two KeyErrors raised in this function, aren't really errors with the key, it's actually that there is actually a sha1 collision.
And so I'd change the errors to custom errors.
What is the likelihood of these? Not very likely but my favorite programmers.SE answer makes me think that ruling this out is not a good idea.
