HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

A minimal HashTable implementation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
minimalimplementationhashtable

Problem

For an assignment, I wrote a minimal HashTable implementation (it only supports additions; no deletions).

To resolve collisions, it uses Linear Probing (required by the assignment).

I'm looking for general feedback regarding style, practice, and these things specifically:

-
I've gotten out of the habit of using null to denote an error/lack of result, since the return type of a method returning null doesn't give any hints about whether or not the method can fail. As a substitute, I've started using Optionals, since its use makes it very clear whether or not the method can fail. There are a few cases in this code where a null would otherwise be used, but I opted for using an Optional. Are these cases a correct usage?:

-
The underlying array is an ArrayList of Optionals of Pairs, since any given cell may or may not contain a value.

-
findNextInsertionPointAfter returns an Optional since the table may be full; in which case there is no next cell to add to.

-
I decided to make my own pair class (KeyValuePair) to store the key-value-pair. Was this the correct approach to take?

-
I figured passing in the hash-function as a lambda to the constructor of HashTable was the most flexible way of setting the function. Is my current set-up optimal?

-
HashTables toString method is my standard concoction when I need to display a sequential container. Unfortunately, it's quite bulky. Is there a better way of setting this up that still gives the neat output?

Sample output from the internal main (at the bottom of HashTable.java):

```
[ -, -, -, -, -, -, -, -, -, -, -, -, - ]
[ -, (1,1), -, -, -, -, -, -, -, -, -, -, - ]
[ -, (1,1), -, -, -, (5,5), -, -, -, -, -, -, - ]
[ -, (1,1), -, -, -, (5,5), -, -, (21,21), -, -, -, - ]
[ (26,26), (1,1), -, -, -, (5,5), -, -, (21,21), -, -, -, - ]
[ (26,26), (1,1), (39,39), -, -, (5,5), -, -, (21,21), -, -, -, - ]
[ (26,26), (1,1), (39,39), (14,14), -, (5,5), -, -, (21,21), -, -, -, - ]
[ (26,26), (1,1),

Solution

As a substitute, I've started using Optionals, since its use makes it very clear whether or not the method can fail.

It tells you about the same as a Nullable annotation would do, just with some overhead.

There are a few cases in this code where a null would otherwise be used, but I opted for using an Optional. Are these cases a correct usage?

I personally agree with this wonderful rant that any use and the mere existence of Optional is wrong.

The underlying array is an ArrayList of Optionals of Pairs, since any given cell may or may not contain a value.

Too bad. That's not what Optional is for. It was meant to be used for return values only (that's why it's not Serializable).

And you added some 12 bytes per entry.

I decided to make my own pair class (KeyValuePair) to store the key-value-pair. Was this the correct approach to take?

Not exactly as there's already Map.Entry.

I figured passing in the hash-function as a lambda to the constructor of HashTable was the most flexible way of setting the function. Is my current set-up optimal?

It adds some overhead, but may make sense.
KeyValuePair

Nothing to say, except for that usually toString would use "=" rather than ",". Just look at a HashMap in debugger.

An immutable KeyValuePair makes a lot of sense for an immutable map, but less for a mutable one. That's why Map.Entry allows to set the value (but not the key).
HashTable

private final ArrayList>> underArr;


By using an ArrayList (which should be declared as List) you're adding additional indirection. The same for Optional. You gain some flexibility, which you can't use.

  • with the size fixed, an array does the job



  • Optional buys you nothing at all



underArr is a terrible name. table would do.

//The user-supplied hash-function. It should take the key, and the size of
// the table, and return a hash.
private final BiFunction hashFunc;


That's a pain. Letting the user specify a Function so that they can choose how to hash their values may make sense. Letting them care about the table size is too bad.

Returning Integer rather than int (see IntFunction) means another efficiency loss.

public HashTable(int tableSize, BiFunction f) {
    hashFunc = f;


So f or hashFunc? Both names are non-optimal (with f being terrible for anything but a local variable), using two different names is a sin.

underArr = new ArrayList>>(tableSize);
    initArr(tableSize);


Without Optional you could save yourself initArr (didn't I say, I hate the name?).

public boolean hasSpace() {
    for (Optional> optPair : underArr) {
        if (!optPair.isPresent()) return true;
    }

    return false;
}


Do you really traverse the whole table on each insert? Otherwise, I can't see what this method is good for.

But if you traverse the whole table on each insert, then it's the final blow to any efficiency you could gain by hashing.

...it's getting too long and I'm getting too negative...

Code Snippets

private final ArrayList<Optional<KeyValuePair<K, V>>> underArr;
//The user-supplied hash-function. It should take the key, and the size of
// the table, and return a hash.
private final BiFunction<K,Integer,Integer> hashFunc;
public HashTable(int tableSize, BiFunction<K,Integer,Integer> f) {
    hashFunc = f;
underArr = new ArrayList<Optional<KeyValuePair<K, V>>>(tableSize);
    initArr(tableSize);
public boolean hasSpace() {
    for (Optional<KeyValuePair<K, V>> optPair : underArr) {
        if (!optPair.isPresent()) return true;
    }

    return false;
}

Context

StackExchange Code Review Q#95046, answer score: 2

Revisions (0)

No revisions yet.