LinkedHashMap as LRU cache I have a simple implementation for a LRU cache using LinkedHashMap. 


I want it to be as generic as   possible. 
This is not for production use, just practice, so I don't care
if its thoroughly robust as far as it
is correct. However, I will welcome
any comments, especially the ones
which might make this better with
simple changes :) 
Are there any other ways of doing this?




class LRUCache&lt;E&gt; {

    @SuppressWarnings("unchecked")
    LRUCache(int size)
    {
        fCacheSize = size;

        // If the cache is to be used by multiple threads,
        // the hashMap must be wrapped with code to synchronize 
        fCacheMap = Collections.synchronizedMap
        (
            //true = use access order instead of insertion order
            new LinkedHashMap&lt;Object,E&gt;(fCacheSize, .75F, true)
            {                                
                @Override
                public boolean removeEldestEntry(Map.Entry eldest)  
                {
                    //when to remove the eldest entry
                    return size() &gt; 99 ;   //size exceeded the max allowed
                }
            }
        );
    }

    public void put(Object key, E elem)
    {
        fCacheMap.put(key, elem);
    }

    public E get(Object key)
    {
        return fCacheMap.get(key);
    }

    private Map&lt;Object,E&gt; fCacheMap;
    private int fCacheSize;
}
First, use generics properly.

Second, why do you track the cache size if when you need it, you get a hardcoded 99? I will assume that was some sort of mistake.

Third, if you really want to annoy most truely professional java developers and course instructors (i.e, not these that write Java code as if they were programming in C#, C++ or Pascal), prefix your class and variable names. If you don't want to do so, do not make this.

Fourth, I would personally prefer to handle synchronization myself. This way, if the class is expanded to include some method that makes two or more operations in the map, I will not break it. I added the atomicGetAndSet method to show this. If I relied in the synchronization provided by the Collections.synchronizedMap instead of my own, the atomicGetAndSet method would not be atomic and it would break if LRUCache instances are used concurrently.

With this in mind, your code became this:

public class LRUCache&lt;K, V&gt; {

    private final Map&lt;K, V&gt; cacheMap;

    public LRUCache(final int cacheSize) {

        // true = use access order instead of insertion order.
        this.cacheMap = new LinkedHashMap&lt;K, V&gt;(cacheSize, 0.75f, true) {                                
            @Override
            protected boolean removeEldestEntry(Map.Entry&lt;K, V&gt; eldest) {
                // When to remove the eldest entry.
                return size() &gt; cacheSize; // Size exceeded the max allowed.
            }
        };
    }

    public synchronized void put(K key, V elem) {
        cacheMap.put(key, elem);
    }

    public synchronized V get(K key) {
        return cacheMap.get(key);
    }

    public synchronized V atomicGetAndSet(K key, V elem) {
        V result = get(key);
        put(key, elem);
        return result;
    }
}