Why doesn't my get-method return the assigned value to my key?

Question

I've been assigned the following task in my Datastructures and Algorithms course, and I have no idea why it does not work.

The task is to implement a Linked Hashtable. I have created two classes, one that acts as the key <-> value pair, representing an element (Entry class), and the Hashtable class that has the methods (currently only put and get), but I can't seem any of them to work. The Hashfunction method was provided by our teachers, so I can't answer any questions regarding that.

Whenever I execute the program, I get no errors, but the list returns empty. Anyone here who can guide towards the right direction of where I am doing wrong? I assume the error lies within the put method, but I can't seem to figure out where to issue might be.

Best regards, Victor

 package Laboration2;

/**
 * A class that works as a container for the key and value.
 * 'Entry' will become an element in our hashtable
 *
 * @author Victor Marante
 * @version 1.0
 * @since 2016-09-22
 */
public class Entry {

    private Object key;
    private Object value;
    private Entry next;

    public Entry(Object key, Object value) {
        this.key = key;
        this.value = value;
    }

    @Override
    public boolean equals(Object obj) {
        Entry keyToCompare = new Entry(obj, null);
        return key.equals(keyToCompare.key);
    }

    public Object getValue() {
        return value;
    }

    public void setValue(Object value) {
        this.value = value;
    }

    public Object getKey() {
        return key;
    }

    public Entry getNext() {
        return next;
    }

    public void setNext(Entry next) {
        this.next = next;
    }
}

Class that holds all the methods for the Hashtable itself:

package Laboration2;

import javax.swing.*;
import java.util.Iterator;
import java.util.LinkedList;

/**
 * Created by Victor on 22/09/16.
 */
public class Hashtable {

    private LinkedList<Object> insertionOrder = new LinkedList<Object>();
    private LinkedList<Entry>[] table;

    // Constructor that initiates a hashtable
    public Hashtable(int size) {
        table = (LinkedList<Entry>[]) new LinkedList<?>[size];
        for (int i = 0; i < size; i++) {
            table[i] = new LinkedList<Entry>();
        }
    }

    // Hashfunction
    private int hashIndex(Object key) {
        int hashCode = key.hashCode();
        hashCode = hashCode % table.length;
        return (hashCode < 0) ? -hashCode : hashCode;
    }

    public Object get(Object key) {
        int hashIndex = hashIndex(key);
        LinkedList<Entry> entries = table[hashIndex];
        Iterator<Entry> it = entries.listIterator();
        while (it.hasNext()) {
            Entry entry = it.next();
            if (entry.equals(key)) {
                return entry.getValue();
            }
        }
        return null;
    }

    public void put(Object key, Object value) {
        int hashIndex = hashIndex(key);
        LinkedList<Entry> entries = table[hashIndex];
        Iterator<Entry> it = entries.listIterator();

        while (it.hasNext()) {
            Entry entry = it.next();
            if (entry.equals(key)) {
                entry.setValue(value);
                insertionOrder.add(value);
            } else {
                entry.setNext(new Entry(key, value));
                insertionOrder.add(value);
            }
        }
    }

    public static void main(String[] args) {

        Hashtable table = new Hashtable(15);
        table.put("hej", "hello");
        table.put("nej", "no");
        table.put("senare", "later");
        table.put("idag", "today");
        table.put("igår", "yesterday");

        table.get("hej");
    }
}

EDIT1 (for Krishas comment):

public void put(Object key, Object value) {
        int hashIndex = hashIndex(key);
        LinkedList<Entry> entries = table[hashIndex];
        Iterator<Entry> it = entries.listIterator();

        if (table[hashIndex] == null) {
            table[hashIndex] = new LinkedList<Entry>(key, value);

        } else {

            while (it.hasNext()) {
                Entry entry = it.next();
                if (entry.equals(key)) {
                    entry.setValue(value);
                    insertionOrder.add(value);
                } else {
                    entry.setNext(new Entry(key, value));
                    insertionOrder.add(value);
                }
            }
        }
    }

Answer 1

What is wrong

That is indeed because of your put method. Your call to setNext is in the wrong place, and this has 2 consequences:

• since the list is initially empty, it.hasNext() will return false, and you will never add anything to the list
• even if you managed to add something to the list, you are calling setNext only if the first key in the list doesn't match. So you are always discarding the second element.

I think some of your I clarity comes from your confusing the two kinds of lists you are handling here: one is the lists inside the table, whose purpose is to handle collisions, meaning different keys ending up in same index of table. The other one is the global list, whose purpose is to record the order of insertion.

For the first type, you don't need 'setNext', you only need to 'add' to it. SetNext is actually meant for the secon type (see below).

What you should do

You should add a new entry only if after processing the whole list there is no match (which also includes the case where the list is empty), meaning after your while loop.

Other notes on your code

• you can simplify your iteration on your list by using the for-each statement. Instead of writing

  Iterator<Entry> it = list.iterator();
  Entry entry;
  while(it.hasNext()){
     entry = it.next();
  

  You can write

  for(Entry entry : list){
  

• it seems to me that your code as it is posted doesn't compile, since you are redefining the variable entry several times. You should define it outside the loop, and only assign values to it in the loop.

• as mentioned by others, the equals method of your Entry class is uselessly complex. You can replace your code by:

  return key.equals(obj);
  

• you don't need your insertionOrder list. The whole point of having a next field in your Entry class, is to be able to link entries so you can iterate on them based on insertion order. All you need to record is the head of the list (the first Entry as well as the tail of it (the latest inserted Entry), so you can link from it.

End result for your put method

public void put(Object key, Object value) {
    int hashIndex = hashIndex(key);
    LinkedList<Entry> entries = table[hashIndex];

    for(Entry entry : entries) {
        if (entry.equals(key)) {
            entry.setValue(value);
            // You might want to update listTail here too
            return;
        }
    }
    Entry newEntry = new Entry(key, value);
    entries.add(newEntry);
    listTail.setNext(newEntry);
    listTail = newEntry;
}

Answer 2

You could do something like this(I commented to make it clearer):

    public void put(Object key, Object value) {
    if (get(key) == null) { //If the key does not exist
        int hash = hashIndex(key); //Hash the key
        Entry<Object> e = new Entry<Object>(key, value); //create new Entry
        table[hash].add(e); //At the hashed index, add new entry,
        insertionOrder.add(value); //Add value to insertionOrder
    }
}

Answer 3

Your put method doesn't insert any element to the table, Because there is no element in the linkedList in the initial stage.

final int hashIndex = hashIndex(key);
    final LinkedList<Entry> entries = table[hashIndex];   //Here entries will be empty on first insert
    final Iterator<Entry> it = entries.listIterator();
while (it.hasNext()) {    // it.hasNext() will returns false, so loop exits without inserting value to the table
        final Entry entry = it.next();
        if (entry.equals(key)) {
            entry.setValue(value);
            insertionOrder.add(value);
        } else {
            entry.setNext(new Entry(key, value));
            insertionOrder.add(value);
        }
    }

In this code the table will be empty when you insert the first element. so the iteration will exit without inserting anything to the table. Hence get returns null.

You can use something like this,

public void put(final Object key, final Object value) {
    final int hashIndex = hashIndex(key);
    final LinkedList<Entry> entries = table[hashIndex];
    final Iterator<Entry> it = entries.listIterator();
    //
    if (!it.hasNext()) {
        entries.add(new Entry(key, value));
    } else {

        while (it.hasNext()) {
            final Entry entry = it.next();
            if (entry.equals(key)) {
                entry.setValue(value);
                insertionOrder.add(value);
            } else {
                entry.setNext(new Entry(key, value));
                insertionOrder.add(value);
            }
        }
    }
}

Please note this is just for the current issue, I'm not sure about your intention.