Skip to main content
updates based on comments
Source Link
Gareth Rees
  • 49.7k
  • 3
  • 129
  • 210

Some very quick notes.

  1. The algorithm in remove is incorrect — it can leave keys unfindable.

This is a common mistake! In The Art of Computer Programming, Knuth comments, "Many computer programmers have great faith in algorithms, and they are surprised to find that the obvious way to delete records from a hash table doesn't work." (Vol. III, p. 533.)

If you want to get this right, Knuth gives an algorithm (pp. 533–4) for deletion in a open hash table with linear probing.

But a simpler approach is to replace the deleted key with a special key meaning "there was a key here but it was deleted" which you later remove when growing or shrinking the table.

Update: To create a unique key that you can be sure won't collide with any key passed by the user, just call object:

    _DELETED = object()  # slot had a key but it was deleted
  1. The use of copy.deepcopy in the grow and shrink methods is wrong — this will make deep copies of the keys and values in the hash table, which isn't what you want (the caller will expect to be able to retrieve the actual item they stored in the table, not some copy of it) and might invalidate the hashes of the keys. You need a shallow copy instead.

  2. It looks as though I can't use None as a key. This seems unsatisfactory.

Update: You asked for use cases. Well, consider using a hash table to memoize a function that might take None as an argument (like functools.lru_cache). Or using a hash table to count the number of occurrences of each element of an iterable (like collections.Counter). Also it's nice to be able to document that any hashable object can be used as a key, without having to mention any special cases.

It's quite straightforward to implement. Wherever the code has None to mean an empty slot, use some other unique object instead:

    _EMPTY = object()    # slot is empty
  1. The rehash method is always called with len(self.slots) as the second argument, so there's no need for that argument — the method can just as well compute the size.

  2. The get and put methods are redundant: you could just use __getitem__ and __setitem__.

  3. The grow and shrink methods are almost identical. This common code could be shared.

  4. del newHashTable is unnecessary — when the method returns, the last reference to newHashTable disappears and it will be collected in any case.

  5. There's lots of duplicated code between the __init__ and clearTable methods — why not have __init__ call clearTable?

Some very quick notes.

  1. The algorithm in remove is incorrect — it can leave keys unfindable.

This is a common mistake! In The Art of Computer Programming, Knuth comments, "Many computer programmers have great faith in algorithms, and they are surprised to find that the obvious way to delete records from a hash table doesn't work." (Vol. III, p. 533.)

If you want to get this right, Knuth gives an algorithm (pp. 533–4) for deletion in a open hash table with linear probing.

But a simpler approach is to replace the deleted key with a special key meaning "there was a key here but it was deleted" which you later remove when growing or shrinking the table.

  1. The use of copy.deepcopy in the grow and shrink methods is wrong — this will make deep copies of the keys and values in the hash table, which isn't what you want (the caller will expect to be able to retrieve the actual item they stored in the table, not some copy of it) and might invalidate the hashes of the keys. You need a shallow copy instead.

  2. It looks as though I can't use None as a key. This seems unsatisfactory.

  3. The rehash method is always called with len(self.slots) as the second argument, so there's no need for that argument — the method can just as well compute the size.

  4. The get and put methods are redundant: you could just use __getitem__ and __setitem__.

  5. The grow and shrink methods are almost identical. This common code could be shared.

  6. del newHashTable is unnecessary — when the method returns, the last reference to newHashTable disappears and it will be collected in any case.

  7. There's lots of duplicated code between the __init__ and clearTable methods — why not have __init__ call clearTable?

Some very quick notes.

  1. The algorithm in remove is incorrect — it can leave keys unfindable.

This is a common mistake! In The Art of Computer Programming, Knuth comments, "Many computer programmers have great faith in algorithms, and they are surprised to find that the obvious way to delete records from a hash table doesn't work." (Vol. III, p. 533.)

If you want to get this right, Knuth gives an algorithm (pp. 533–4) for deletion in a open hash table with linear probing.

But a simpler approach is to replace the deleted key with a special key meaning "there was a key here but it was deleted" which you later remove when growing or shrinking the table.

Update: To create a unique key that you can be sure won't collide with any key passed by the user, just call object:

    _DELETED = object()  # slot had a key but it was deleted
  1. The use of copy.deepcopy in the grow and shrink methods is wrong — this will make deep copies of the keys and values in the hash table, which isn't what you want (the caller will expect to be able to retrieve the actual item they stored in the table, not some copy of it) and might invalidate the hashes of the keys. You need a shallow copy instead.

  2. It looks as though I can't use None as a key. This seems unsatisfactory.

Update: You asked for use cases. Well, consider using a hash table to memoize a function that might take None as an argument (like functools.lru_cache). Or using a hash table to count the number of occurrences of each element of an iterable (like collections.Counter). Also it's nice to be able to document that any hashable object can be used as a key, without having to mention any special cases.

It's quite straightforward to implement. Wherever the code has None to mean an empty slot, use some other unique object instead:

    _EMPTY = object()    # slot is empty
  1. The rehash method is always called with len(self.slots) as the second argument, so there's no need for that argument — the method can just as well compute the size.

  2. The get and put methods are redundant: you could just use __getitem__ and __setitem__.

  3. The grow and shrink methods are almost identical. This common code could be shared.

  4. del newHashTable is unnecessary — when the method returns, the last reference to newHashTable disappears and it will be collected in any case.

  5. There's lots of duplicated code between the __init__ and clearTable methods — why not have __init__ call clearTable?

added 10 characters in body
Source Link
Gareth Rees
  • 49.7k
  • 3
  • 129
  • 210

Some very quick notes.

  1. The algorithm in remove is incorrect — it can leave keys unfindable.

This is a common mistake! In The Art of Computer Programming, Knuth comments, "Many computer programmers have great faith in algorithms, and they are surprised to find that the obvious way to delete records from a hash table doesn't work." (Vol. III, p. 533.)

If you want to get this right, Knuth gives an algorithm (pp. 533–4) for deletion in a open hash table with linear probing.

But a simpler approach is to replace the deleted key with a special key meaning "there was a key here but it was deleted" which you later remove when growing or shrinking the table.

  1. The use of copy.deepcopy in the grow and shrink methods is wrong — this will make deep copies of the keys and values in the hash table, which isn't what you want (the caller will expect to be able to retrieve the actual item they stored in the table, not some copy of it) and might invalidate the hashes of the keys. You need a shallow copy instead.

  2. It looks as though I can't use None as a key. This seems unsatisfactory.

  3. The rehash method is always called with len(self.slots) as the second argument, so there's no need for that argument — the method can just as well compute the size.

  4. The get and put methods are redundant: you could just use __getitem__ and __setitem__.

  5. The grow and shrink methods are almost identical. This common code could be shared.

  6. del newHashTable is unnecessary — when the method returns, the last reference to newHashTable disappears and it will be collected in any case.

  7. There's lots of duplicated code between the __init__ and clearclearTable methods — why not have __init__ call clearclearTable?

Some very quick notes.

  1. The algorithm in remove is incorrect — it can leave keys unfindable.

This is a common mistake! In The Art of Computer Programming, Knuth comments, "Many computer programmers have great faith in algorithms, and they are surprised to find that the obvious way to delete records from a hash table doesn't work." (Vol. III, p. 533.)

If you want to get this right, Knuth gives an algorithm (pp. 533–4) for deletion in a open hash table with linear probing.

But a simpler approach is to replace the deleted key with a special key meaning "there was a key here but it was deleted" which you later remove when growing or shrinking the table.

  1. The use of copy.deepcopy in the grow and shrink methods is wrong — this will make deep copies of the keys and values in the hash table, which isn't what you want (the caller will expect to be able to retrieve the actual item they stored in the table, not some copy of it) and might invalidate the hashes of the keys. You need a shallow copy instead.

  2. It looks as though I can't use None as a key. This seems unsatisfactory.

  3. The rehash method is always called with len(self.slots) as the second argument, so there's no need for that argument — the method can just as well compute the size.

  4. The get and put methods are redundant: you could just use __getitem__ and __setitem__.

  5. The grow and shrink methods are almost identical. This common code could be shared.

  6. del newHashTable is unnecessary — when the method returns, the last reference to newHashTable disappears and it will be collected in any case.

  7. There's lots of duplicated code between the __init__ and clear methods — why not have __init__ call clear?

Some very quick notes.

  1. The algorithm in remove is incorrect — it can leave keys unfindable.

This is a common mistake! In The Art of Computer Programming, Knuth comments, "Many computer programmers have great faith in algorithms, and they are surprised to find that the obvious way to delete records from a hash table doesn't work." (Vol. III, p. 533.)

If you want to get this right, Knuth gives an algorithm (pp. 533–4) for deletion in a open hash table with linear probing.

But a simpler approach is to replace the deleted key with a special key meaning "there was a key here but it was deleted" which you later remove when growing or shrinking the table.

  1. The use of copy.deepcopy in the grow and shrink methods is wrong — this will make deep copies of the keys and values in the hash table, which isn't what you want (the caller will expect to be able to retrieve the actual item they stored in the table, not some copy of it) and might invalidate the hashes of the keys. You need a shallow copy instead.

  2. It looks as though I can't use None as a key. This seems unsatisfactory.

  3. The rehash method is always called with len(self.slots) as the second argument, so there's no need for that argument — the method can just as well compute the size.

  4. The get and put methods are redundant: you could just use __getitem__ and __setitem__.

  5. The grow and shrink methods are almost identical. This common code could be shared.

  6. del newHashTable is unnecessary — when the method returns, the last reference to newHashTable disappears and it will be collected in any case.

  7. There's lots of duplicated code between the __init__ and clearTable methods — why not have __init__ call clearTable?

Source Link
Gareth Rees
  • 49.7k
  • 3
  • 129
  • 210

Some very quick notes.

  1. The algorithm in remove is incorrect — it can leave keys unfindable.

This is a common mistake! In The Art of Computer Programming, Knuth comments, "Many computer programmers have great faith in algorithms, and they are surprised to find that the obvious way to delete records from a hash table doesn't work." (Vol. III, p. 533.)

If you want to get this right, Knuth gives an algorithm (pp. 533–4) for deletion in a open hash table with linear probing.

But a simpler approach is to replace the deleted key with a special key meaning "there was a key here but it was deleted" which you later remove when growing or shrinking the table.

  1. The use of copy.deepcopy in the grow and shrink methods is wrong — this will make deep copies of the keys and values in the hash table, which isn't what you want (the caller will expect to be able to retrieve the actual item they stored in the table, not some copy of it) and might invalidate the hashes of the keys. You need a shallow copy instead.

  2. It looks as though I can't use None as a key. This seems unsatisfactory.

  3. The rehash method is always called with len(self.slots) as the second argument, so there's no need for that argument — the method can just as well compute the size.

  4. The get and put methods are redundant: you could just use __getitem__ and __setitem__.

  5. The grow and shrink methods are almost identical. This common code could be shared.

  6. del newHashTable is unnecessary — when the method returns, the last reference to newHashTable disappears and it will be collected in any case.

  7. There's lots of duplicated code between the __init__ and clear methods — why not have __init__ call clear?