-1
\$\begingroup\$

I implemented a LRU-cache in C# and wanted to hear suggestions in making it more readable and maybe even more compact.

public class LRUCache<K, V>
{
    public int Capacity { get; }
    Dictionary<K, V> cache;
    K[] keyRingBuffer;
    int ringBufferIndex;

    int LRUIndex => (ringBufferIndex + 1) % Capacity;
    K LRU => keyRingBuffer[LRUIndex];

    public LRUCache(int capacity)
    {
        Capacity = capacity;
        cache = new Dictionary<K, V>(Capacity);
        keyRingBuffer = new K[Capacity];
        ringBufferIndex = 0;
    }

    public bool Contains(K key) => cache.ContainsKey(key);
    public int Size => cache.Count;

    public V Get(K key)
    {
        if (cache.TryGetValue(key, out var value))
        {
            keyRingBuffer[LRUIndex] = key;
            ringBufferIndex = LRUIndex;
            return value;
        }
        throw new Exception($"Element for key {key} not in cache");
    }

    public void Add(K key, V val)
    {
        if (cache.Count >= Capacity) cache.Remove(LRU);

        ringBufferIndex = LRUIndex;
        keyRingBuffer[ringBufferIndex] = key;
        cache.Add(key, val);
    }
}

I would love it to be more like -- how Robert Martin says -- "well written prose".

\$\endgroup\$
  • \$\begingroup\$ Looks like you are evicting the “most” recently used element, not least recently used. \$\endgroup\$ – Stack crashed Oct 14 '17 at 15:34
1
\$\begingroup\$

First make it right. Then worry about readability, compactness, etc.

Consider the following use case:

var foos = new LRUCache<Foo, Bar>(2);
foos.Add(foo1, bar1);
foos.Add(foo2, bar2);
Quux(foos.Get(foo1));
Quux(foos.Get(foo1));
// The least recently used key is foo2, so it should be evicted
foos.Add(foo3, bar3);
Assert(foos.Contains(foo1));

Why does it fail?

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.