From f3ead5f5a480117d614af5aabff5555d1c15028d Mon Sep 17 00:00:00 2001 From: William Fiset Date: Fri, 3 Apr 2026 08:38:06 -0700 Subject: [PATCH] Refactor HashTableSeparateChaining: encapsulate Entry, fix iterator, simplify API Make Entry a private static inner class. Remove redundant method aliases (add/insert/hasKey) in favor of put/containsKey. Add dedicated modCount for fail-fast iteration instead of using size as a proxy. Rewrite iterator so hasNext() is idempotent. Use single-pass iterator removal in removeEntry to avoid double linear scan. Remove unnecessary bucket.clear() in resize. Add educational comments on key implementation details. Co-Authored-By: Claude Opus 4.6 --- .../hashtable/HashTableSeparateChaining.java | 289 +++++++++--------- .../HashTableSeparateChainingTest.java | 26 +- 2 files changed, 149 insertions(+), 166 deletions(-) diff --git a/src/main/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableSeparateChaining.java b/src/main/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableSeparateChaining.java index 3de53deb6..b954f1b75 100644 --- a/src/main/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableSeparateChaining.java +++ b/src/main/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableSeparateChaining.java @@ -1,5 +1,9 @@ /** - * An implementation of a hash-table using separate chaining with a linked list. + * A hash table implementation using separate chaining with linked lists. + * + * Each bucket holds a linked list of entries. On collision, new entries are appended to the list. + * When the load factor threshold is exceeded, the table doubles in capacity and all entries are + * rehashed. * * @author William Fiset, william.alexandre.fiset@gmail.com */ @@ -7,46 +11,38 @@ import java.util.*; -class Entry { - - int hash; - K key; - V value; +@SuppressWarnings("unchecked") +public class HashTableSeparateChaining implements Iterable { - public Entry(K key, V value) { - this.key = key; - this.value = value; - this.hash = key.hashCode(); - } + private static class Entry { + int hash; + K key; + V value; - @Override - public int hashCode() { - return hash; - } + // Cache the hash code so we don't recompute it on every resize + Entry(K key, V value) { + this.key = key; + this.value = value; + this.hash = key.hashCode(); + } - @Override - public boolean equals(Object obj) { - if (this == obj) return true; - if (obj == null || getClass() != obj.getClass()) return false; - Entry other = (Entry) obj; - if (hash != other.hash) return false; - return key.equals(other.key); - } + boolean keyEquals(K other) { + return key.equals(other); + } - @Override - public String toString() { - return key + " => " + value; + @Override + public String toString() { + return key + " => " + value; + } } -} - -@SuppressWarnings("unchecked") -public class HashTableSeparateChaining implements Iterable { private static final int DEFAULT_CAPACITY = 3; private static final double DEFAULT_LOAD_FACTOR = 0.75; private double maxLoadFactor; - private int capacity, threshold, size = 0; + private int capacity, threshold, size; + // Tracks structural modifications for fail-fast iteration + private int modCount; private LinkedList>[] table; public HashTableSeparateChaining() { @@ -57,9 +53,9 @@ public HashTableSeparateChaining(int capacity) { this(capacity, DEFAULT_LOAD_FACTOR); } - // Designated constructor public HashTableSeparateChaining(int capacity, double maxLoadFactor) { - if (capacity < 0) throw new IllegalArgumentException("Illegal capacity"); + if (capacity < 0) + throw new IllegalArgumentException("Illegal capacity"); if (maxLoadFactor <= 0 || Double.isNaN(maxLoadFactor) || Double.isInfinite(maxLoadFactor)) throw new IllegalArgumentException("Illegal maxLoadFactor"); this.maxLoadFactor = maxLoadFactor; @@ -68,210 +64,197 @@ public HashTableSeparateChaining(int capacity, double maxLoadFactor) { table = new LinkedList[this.capacity]; } - // Returns the number of elements currently inside the hash-table public int size() { return size; } - // Returns true/false depending on whether the hash-table is empty public boolean isEmpty() { return size == 0; } - // Converts a hash value to an index. Essentially, this strips the - // negative sign and places the hash value in the domain [0, capacity) + // Strips the negative sign and maps the hash into [0, capacity) private int normalizeIndex(int keyHash) { return (keyHash & 0x7FFFFFFF) % capacity; } - // Clears all the contents of the hash-table public void clear() { Arrays.fill(table, null); size = 0; + modCount++; } public boolean containsKey(K key) { - return hasKey(key); - } - - // Returns true/false depending on whether a key is in the hash table - public boolean hasKey(K key) { int bucketIndex = normalizeIndex(key.hashCode()); - return bucketSeekEntry(bucketIndex, key) != null; + return seekEntry(bucketIndex, key) != null; } - // Insert, put and add all place a value in the hash-table public V put(K key, V value) { - return insert(key, value); - } - - public V add(K key, V value) { - return insert(key, value); - } - - public V insert(K key, V value) { - if (key == null) throw new IllegalArgumentException("Null key"); + if (key == null) + throw new IllegalArgumentException("Null key"); Entry newEntry = new Entry<>(key, value); int bucketIndex = normalizeIndex(newEntry.hash); - return bucketInsertEntry(bucketIndex, newEntry); + return insertEntry(bucketIndex, newEntry); } - // Gets a key's values from the map and returns the value. - // NOTE: returns null if the value is null AND also returns - // null if the key does not exists, so watch out.. public V get(K key) { - if (key == null) return null; + if (key == null) + return null; int bucketIndex = normalizeIndex(key.hashCode()); - Entry entry = bucketSeekEntry(bucketIndex, key); - if (entry != null) return entry.value; - return null; + Entry entry = seekEntry(bucketIndex, key); + return entry != null ? entry.value : null; } - // Removes a key from the map and returns the value. - // NOTE: returns null if the value is null AND also returns - // null if the key does not exists. public V remove(K key) { - if (key == null) return null; + if (key == null) + return null; int bucketIndex = normalizeIndex(key.hashCode()); - return bucketRemoveEntry(bucketIndex, key); + return removeEntry(bucketIndex, key); } - // Removes an entry from a given bucket if it exists - private V bucketRemoveEntry(int bucketIndex, K key) { - Entry entry = bucketSeekEntry(bucketIndex, key); - if (entry != null) { - LinkedList> links = table[bucketIndex]; - links.remove(entry); - --size; - return entry.value; - } else return null; + // Single-pass find-and-remove using an iterator to avoid scanning the bucket twice + private V removeEntry(int bucketIndex, K key) { + LinkedList> bucket = table[bucketIndex]; + if (bucket == null) + return null; + java.util.Iterator> iter = bucket.iterator(); + while (iter.hasNext()) { + Entry entry = iter.next(); + if (entry.keyEquals(key)) { + iter.remove(); + size--; + modCount++; + return entry.value; + } + } + return null; } - // Inserts an entry in a given bucket only if the entry does not already - // exist in the given bucket, but if it does then update the entry value - private V bucketInsertEntry(int bucketIndex, Entry entry) { + // If the key already exists, update its value and return the old value. + // Otherwise, insert a new entry and return null. + private V insertEntry(int bucketIndex, Entry entry) { LinkedList> bucket = table[bucketIndex]; - if (bucket == null) table[bucketIndex] = bucket = new LinkedList<>(); + if (bucket == null) + table[bucketIndex] = bucket = new LinkedList<>(); - Entry existentEntry = bucketSeekEntry(bucketIndex, entry.key); - if (existentEntry == null) { + Entry existing = seekEntry(bucketIndex, entry.key); + if (existing == null) { bucket.add(entry); - if (++size > threshold) resizeTable(); - return null; // Use null to indicate that there was no previous entry - } else { - V oldVal = existentEntry.value; - existentEntry.value = entry.value; - return oldVal; + if (++size > threshold) + resizeTable(); + modCount++; + return null; } + V oldVal = existing.value; + existing.value = entry.value; + return oldVal; } - // Finds and returns a particular entry in a given bucket if it exists, returns null otherwise - private Entry bucketSeekEntry(int bucketIndex, K key) { - if (key == null) return null; + // Linearly scans the bucket's chain looking for a matching key + private Entry seekEntry(int bucketIndex, K key) { + if (key == null) + return null; LinkedList> bucket = table[bucketIndex]; - if (bucket == null) return null; - for (Entry entry : bucket) if (entry.key.equals(key)) return entry; + if (bucket == null) + return null; + for (Entry entry : bucket) { + if (entry.keyEquals(key)) + return entry; + } return null; } - // Resizes the internal table holding buckets of entries + // Doubles the table capacity and rehashes all entries into new buckets private void resizeTable() { capacity *= 2; threshold = (int) (capacity * maxLoadFactor); - LinkedList>[] newTable = new LinkedList[capacity]; - for (int i = 0; i < table.length; i++) { - if (table[i] != null) { - for (Entry entry : table[i]) { - int bucketIndex = normalizeIndex(entry.hash); - LinkedList> bucket = newTable[bucketIndex]; - if (bucket == null) newTable[bucketIndex] = bucket = new LinkedList<>(); - bucket.add(entry); - } - - // Avoid memory leak. Help the GC - table[i].clear(); - table[i] = null; + for (LinkedList> bucket : table) { + if (bucket == null) + continue; + for (Entry entry : bucket) { + int i = normalizeIndex(entry.hash); + if (newTable[i] == null) + newTable[i] = new LinkedList<>(); + newTable[i].add(entry); } } table = newTable; } - // Returns the list of keys found within the hash table public List keys() { - List keys = new ArrayList<>(size()); - for (LinkedList> bucket : table) - if (bucket != null) for (Entry entry : bucket) keys.add(entry.key); + List keys = new ArrayList<>(size); + for (LinkedList> bucket : table) { + if (bucket != null) + for (Entry entry : bucket) + keys.add(entry.key); + } return keys; } - // Returns the list of values found within the hash table public List values() { - List values = new ArrayList<>(size()); - for (LinkedList> bucket : table) - if (bucket != null) for (Entry entry : bucket) values.add(entry.value); + List values = new ArrayList<>(size); + for (LinkedList> bucket : table) { + if (bucket != null) + for (Entry entry : bucket) + values.add(entry.value); + } return values; } - // Return an iterator to iterate over all the keys in this map @Override - public java.util.Iterator iterator() { - final int MODIFICATION_COUNT = size; // Using size as a proxy for modifications for now, but better to have a dedicated counter - return new java.util.Iterator() { + public Iterator iterator() { + return new Iterator() { + final int expectedModCount = modCount; int bucketIndex = 0; - java.util.Iterator> bucketIter = (table[0] == null) ? null : table[0].iterator(); + Iterator> bucketIter = advanceToNextBucket(); - @Override - public boolean hasNext() { - // An item was added or removed while iterating - if (MODIFICATION_COUNT != size) throw new java.util.ConcurrentModificationException(); - - // No iterator or the current iterator is empty - if (bucketIter == null || !bucketIter.hasNext()) { - // Search next buckets until a valid iterator is found - while (++bucketIndex < capacity) { - if (table[bucketIndex] != null) { - // Make sure this iterator actually has elements -_- - java.util.Iterator> nextIter = table[bucketIndex].iterator(); - if (nextIter.hasNext()) { - bucketIter = nextIter; - break; - } - } - } + private Iterator> advanceToNextBucket() { + while (bucketIndex < capacity) { + if (table[bucketIndex] != null && !table[bucketIndex].isEmpty()) + return table[bucketIndex].iterator(); + bucketIndex++; } - return bucketIndex < capacity; + return null; } @Override - public K next() { - if (!hasNext()) throw new NoSuchElementException(); - return bucketIter.next().key; + public boolean hasNext() { + if (expectedModCount != modCount) + throw new ConcurrentModificationException(); + return bucketIter != null && bucketIter.hasNext(); } @Override - public void remove() { - throw new UnsupportedOperationException(); + public K next() { + if (expectedModCount != modCount) + throw new ConcurrentModificationException(); + if (bucketIter == null || !bucketIter.hasNext()) + throw new NoSuchElementException(); + K key = bucketIter.next().key; + if (!bucketIter.hasNext()) { + bucketIndex++; + bucketIter = advanceToNextBucket(); + } + return key; } }; } - // Returns a string representation of this hash table @Override public String toString() { - StringBuilder sb = new StringBuilder(); - sb.append("{"); + StringBuilder sb = new StringBuilder("{"); boolean first = true; - for (int i = 0; i < capacity; i++) { - if (table[i] == null) continue; - for (Entry entry : table[i]) { - if (!first) sb.append(", "); + for (LinkedList> bucket : table) { + if (bucket == null) + continue; + for (Entry entry : bucket) { + if (!first) + sb.append(", "); sb.append(entry); first = false; } } - sb.append("}"); - return sb.toString(); + return sb.append("}").toString(); } } diff --git a/src/test/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableSeparateChainingTest.java b/src/test/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableSeparateChainingTest.java index a943f95e1..07bdd0ea8 100644 --- a/src/test/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableSeparateChainingTest.java +++ b/src/test/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableSeparateChainingTest.java @@ -135,13 +135,13 @@ public void testIterator() { map = new HashTableSeparateChaining<>(); List rand_nums = genRandList(MAX_SIZE); - for (Integer key : rand_nums) assertThat(map.add(key, key)).isEqualTo(map2.put(key, key)); + for (Integer key : rand_nums) assertThat(map.put(key, key)).isEqualTo(map2.put(key, key)); int count = 0; for (Integer key : map) { assertThat(map.get(key)).isEqualTo(key); assertThat(map.get(key)).isEqualTo(map2.get(key)); - assertThat(map.hasKey(key)).isTrue(); + assertThat(map.containsKey(key)).isTrue(); assertThat(rand_nums.contains(key)).isTrue(); count++; } @@ -163,10 +163,10 @@ public void testConcurrentModificationException() { assertThrows( ConcurrentModificationException.class, () -> { - map.add(1, 1); - map.add(2, 1); - map.add(3, 1); - for (Integer key : map) map.add(4, 4); + map.put(1, 1); + map.put(2, 1); + map.put(3, 1); + for (Integer key : map) map.put(4, 4); }); } @@ -175,9 +175,9 @@ public void testConcurrentModificationException2() { assertThrows( ConcurrentModificationException.class, () -> { - map.add(1, 1); - map.add(2, 1); - map.add(3, 1); + map.put(1, 1); + map.put(2, 1); + map.put(3, 1); for (Integer key : map) map.remove(2); }); } @@ -245,10 +245,10 @@ public void removeTestComplex1() { HashObject o3 = new HashObject(88, 3); HashObject o4 = new HashObject(88, 4); - map.add(o1, 111); - map.add(o2, 111); - map.add(o3, 111); - map.add(o4, 111); + map.put(o1, 111); + map.put(o2, 111); + map.put(o3, 111); + map.put(o4, 111); map.remove(o2); map.remove(o3);