LRU Cache in Java

Question

Problem description:

Design and implement a data structure for Least Recently Used (LRU) cache. It should support the following operations: get and put.

get(key) - Get the value (will always be positive) of the key if the key exists in the cache, otherwise return -1. put(key, value) - Set or insert the value if the key is not already present. When the cache reached its capacity, it should invalidate the least recently used item before inserting a new item.

Follow up: Could you do both operations in O(1) time complexity?

I passed 17/18 test cases with this and failed the last one due to this exceeding time constraints. I'm guessing something here isn't O(1)? I've spent hours but can't identify it.

class LRUCache {
 Map<Integer, Integer> cache;
 Queue<Integer> q;
 int capacity;
 public LRUCache(int capacity) {
 cache = new HashMap<>();
 q = new LinkedList<Integer>();
 this.capacity = capacity;
 }
 public int get(int key) {
 if (cache.get(key) == null || cache.get(key) == -1) return -1;
 int value = cache.get(key);
 q.remove(key);
 q.add(key);
 System.out.println("get() - key: " + key + " value: " + value);
 return value;
 }
 public void put(int key, int value) {
 if (cache.get(key) == null || cache.get(key) == -1) {
 if (q.size() >= capacity) {
 evict();
 }
 } else {
 q.remove(key);
 }
 q.add(key);
 cache.put(key, value);
 System.out.println("put()...key: " + key + " queue size: " + q.size());
 }
 private void evict() {
 int toRemove = q.remove();
 cache.put(toRemove, -1);
 System.out.println("Evict: " + toRemove + " queue size: " + q.size());
 }
}
/**
 * Your LRUCache object will be instantiated and called as such:
 * LRUCache obj = new LRUCache(capacity);
 * int param_1 = obj.get(key);
 * obj.put(key,value);
 */

Answer 1

class LRUCache {
 Map<Integer, Integer> cache;
 Queue<Integer> q;
 int capacity;

Is there any reason for not making these fields private?

---

 public int get(int key) {
 if (cache.get(key) == null || cache.get(key) == -1) return -1;

Why the special case if cache.get(key) == -1? I don't see that in the spec.

 int value = cache.get(key);

This method has now called cache.get(key) three times. I think this could be optimised somewhat...

 q.remove(key);

This is not O(1). I suspect that you will need to implement your own linked list to get O(1) removal from the middle.

 System.out.println("get() - key: " + key + " value: " + value);

I would recommend removing debug printing before asking for code review.

---

 public void put(int key, int value) {

My observations are similar to those on get.

---

 private void evict() {
 int toRemove = q.remove();
 cache.put(toRemove, -1);

Huh? This answers my earlier question about special cases, but raises a more fundamental question. Do you understand what the point of an LRU cache is? You should never have cache.size() > capacity.