[en4bz]

Not really sure why they reimplemented std::list. Also that NoCopy base class adds a vptr for no reason. And so do all those other virtual destructors. I personally wouldn't trust this code at first glance.

[mohaps]

the kv::List is not intended as a generic linked list implementation like std::list.

The reason for the specific implementation is to get constant time removal and remove-add_to_end operations. Truth be told, it is a hold-over from the previous implementation. I shall be writing benchmarks for this next up and shall revisit the decision on whether or not to use std::list soon-ish :)

The : private NoCopy is just to block the copy constructors.

[quinnftw]

Personally I would use a macro such as:

    #define DISALLOW_COPY_AND_ASSIGN(TypeName) \
    TypeName(TypeName&) = delete;              \
    void operator=(TypeName) = delete;

Introducing a virtual base class for disabling copying introduces a vtable and un-needed inheritance.

[_lbaq]

>The : private NoCopy is just to block the copy constructors.

If its C++11 why not just use the delete keyword to prevent auto generating members ?

[Someone]

"The reason for the specific implementation is to get constant time removal and remove-add_to_end operations"

http://en.cppreference.com/w/cpp/container/list:

"std::list is a container that supports constant time insertion and removal of elements from anywhere in the container."

'The : private NoCopy is just to block the copy constructors."

But does it need a virtual destructor?

[en4bz]

You could have used std::list iterators instead of raw pointers to a node in the map to implement that.

[mohaps]

now i remember why I went with the old linked list implementation.

with the std::list, everytime I refreshed a node... i did a copy.

basically to the tune of list.push_back(*iter) where as I wanted to keep it as a simple unlink and relink of the node

copy of the comment: https://news.ycombinator.com/item?id=12391069

[en4bz]

To swap nodes in the list you need to use splice like this:

keys_.splice(keys_.begin(), keys_, iter->second);

Where iter is from the cache lookup.

[mohaps]

yeah, saw the gist posted by quinnftw. that actually is a better way to do what I'm doing. Will update that along with the

cache.remove(const Key& k, F deleteCallback) method.

Thanks for the feedback