HashTable implementation

I have been trying to implement the Hash Table function in Java and here is what I came up with. I would just like to get an opinion on this work. Would there be a better way or any improvement to make to this code ?

HashTable.java : basically contains all the hastable functions to create the table, add a node and retrieve a node

```
import java.math.BigInteger;

public class HashMap {
// Srtting table size to a max of 32, value used to modulus for hash value.
private final static int TABLE_SIZE = 32;

HashEntry[] table;

HashMap() {
table = new HashEntry[TABLE_SIZE];
for (int i = 0; i < TABLE_SIZE; i++)
table[i] = null;
}

/ function to retrieve value from the table according to key /
public int get(String key) {
int hash = new BigInteger(toAscii(key)).mod(new BigInteger(((Integer)TABLE_SIZE).toString())).intValue();
while (table[hash] != null && table[hash].getKey() != key)
hash = (hash + 1) % TABLE_SIZE;
if (table[hash] == null)
return -1;
else
return table[hash].getValue();
}

/ function to add value to the table /
public void put(String key, int value) {
//creating hash code using key value given as a string
int hash = new BigInteger(toAscii(key)).mod(new BigInteger(((Integer)TABLE_SIZE).toString())).intValue();
while (table[hash] != null && table[hash].getKey() != key)
hash = (hash + 1) % TABLE_SIZE;
table[hash] = new HashEntry(key, value);
}

/ value to create the Hash code from he name entered, basically converting name to ASCII /
public static String toAscii(String s){
StringBuilder sb = new StringBuilder();
long asciiInt;
// loop through all values in the string, including blanks
for (int i = 0; i < s.length(); i++){

Implementing a hash table

This hash table implementation is a bit limited: it supports only String keys and int values. It would be good to generalize it.

When getting a value of a key not in the table, the common expected behavior is null. Since you're using int as the type of values, this is not possible, but -1 just doesn't seem special enough to be commonly understood as "missing value".

The toAscii method is very bad:

• It's only used internally, so it shouldn't be public

• The name doesn't describe very well what it does: converting a string to ascii sounds more like an encoding transformation than a hash code calculation. It would have been better to call it calculateHashCode, and make it return a BigInteger

• Even better would have been to use String's very own hashCode, rather than reimplementing your own

Naming

HashMap entry = new HashMap();

"entry" is an inappropriate name for a map. "map" would seem the obvious choice.

General coding issues

The HashEntry class is an implementation detail of your hash table.
As such, it would be best to hide this class,
by making it a private static class inside the hash table.

Instead of this:

new BigInteger(((Integer)TABLE_SIZE).toString());

A much better and simpler way:

BigInteger.valueOf(TABLE_SIZE);

Limit variables to the smallest scope possible,
to prevent accidental changes outside their intended purpose.
For example in this code:

long asciiInt;
      // loop through all values in the string, including blanks
      for (int i = 0; i < s.length(); i++){
          //getting Ascii value of character and adding it to the string.
          char c = s.charAt(i);
          asciiInt = (int)c; 
          sb.append(asciiInt);
      }

asciiInt should have been declared inside the loop.

This code looks very confused:
you get a char, cast it to an int to store in a long variable.
This could have been simply:

for (int i = 0; i < s.length(); i++){
        sb.append((int) s.charAt(i));
    }

And even simpler with a for-each loop:

for (char c : s.toCharArray()) {
        sb.append((int) c);
    }

table = new HashEntry[TABLE_SIZE];
        for (int i = 0; i < TABLE_SIZE; i++)
              table[i] = null;

When iterating over all elements of an array,
I recommend using the array's length as the limit.
It's safer. Like this:

table = new HashEntry[TABLE_SIZE];
        for (int i = 0; i < table.length; i++) { ... }

But since you're only using this loop to assign the values to null,
the entire loop is pointless, you can safely delete it.

It's highly suspicious when you compare objects using != like you do for the keys in this code:

while (table[hash] != null && table[hash].getKey() != key)
        hash = (hash + 1) % TABLE_SIZE;

For example, what do you think this code will print:

String key1 = new String("Jack");
    String key2 = new String("Jack");
    entry.put(key1, 11);
    entry.put(key2, 21);
    System.out.println(entry.get(key1));
    System.out.println(entry.get(key2));

It will print 11 and 21. I suggest to replace != on objects everywhere with .equals(...). Then these two keys will be considered equal, as usually expected from a hash map, and the print statements will return 21 and 21.

Other coding style issues

• I suggest using braces { ... } even for single-statement blocks

-
Instead of comments like / function to retrieve value from the table according to key /, use proper JavaDoc, for example:

/**
 * Retrieve value from the table according to key
 * 
 * @param key the key to look for
 * @return the value of the key, or null if doesn't exist
 */

-
Since the fields of HashEntry never change, you can make them final

-
This code block appears twice:

while (table[hash] != null && !table[hash].getKey().equals(key)) 
    hash = (hash + 1) % TABLE_SIZE;

Avoid code duplication. Extract to a private helper method,
or refactor your implementation in a way that you don't have to duplicate code.

Same note for this code:

int hash = new BigInteger(toAscii(key)).mod(new BigInteger(((Integer)TABLE_SIZE).toString())).intValue();

Improved implementation

Taking some of the suggestions above,
the implementation can be simplified and improved:

private int calculateHashCode(String key) {
    int mod = key.hashCode() % TABLE_SIZE;
    return mod < 0 ? mod + TABLE_SIZE : mod;
}

private int findIndex(String key) {
    int index = calculateHashCode(key);
    while (table[index] != null && !table[index].getKey().equals(key)) {
        index = (index + 1) % TABLE_SIZE;
    }
    return index;
}

public int get(String key) {
    int index = findIndex(key);
    return table[index] == null ? -1 : table[index].getValue();
}

public void put(String key, int value) {
    table[findIndex(key)] = new HashEntry(key, value);
}

StackExchange Code Review Q#73542, answer score: 17