A little code clean up.

1 files changed, 72 insertions, 32 deletions

diff --git a/pylru.py b/pylru.py
index f135d9f..e76e39e 100644
--- a/pylru.py
+++ b/pylru.py
@@ -42,8 +42,6 @@ class _dlnode(object):
         self.key = None
 
 
-
-
 class lrucache(object):
 
     def __init__(self, size, callback=None):
@@ -54,8 +52,8 @@ class lrucache(object):
 
 
         # Initalize the list with one empty node. Create a node and
-        # assign it to the 'head' variable, which represents the head node in the
-        # list.
+        # assign it to the 'head' variable, which represents the head node in
+        # the list.
         self.head = _dlnode()
         self.head.next = self.head
         self.head.prev = self.head
@@ -71,19 +69,18 @@ class lrucache(object):
 
     # Does not call callback to write any changes!
     def clear(self):
-        self.table.clear()
-
-        node = self.head
-        for i in range(self.listSize):
+        for node in self.dli():
             node.key = None
             node.obj = None
-            node = node.next
+
+        self.table.clear()
 
 
     def __contains__(self, key):
-        return key in self.table
         # XXX Should this move the object to front of list? XXX
+        return key in self.table
 
+    # Looks up a value in the cache without affecting cache order.
     def peek(self, key):
         # Look up the node
         node = self.table[key]
@@ -91,13 +88,12 @@ class lrucache(object):
 
 
     def __getitem__(self, key):
-
         # Look up the node
         node = self.table[key]
 
-        # Update the list ordering. Move this node so that is directly proceeds the
-        # head node. Then set the 'head' variable to it. This makes it the new head
-        # of the list.
+        # Update the list ordering. Move this node so that is directly proceeds
+        # the head node. Then set the 'head' variable to it. This makes it the
+        # new head of the list.
         self.mtf(node)
         self.head = node
 
@@ -106,9 +102,8 @@ class lrucache(object):
 
 
     def __setitem__(self, key, obj):
-
-        # First, see if any object is stored under 'key' in the cache already. If
-        # so we are going to replace that object with the new one.
+        # First, see if any object is stored under 'key' in the cache already.
+        # If so we are going to replace that object with the new one.
         if key in self.table:
 
             # Lookup the node
@@ -140,7 +135,7 @@ class lrucache(object):
         # If the node already contains something we need to remove the old key from
         # the dictionary.
         if node.key is not None:
-            if self.callback:
+            if self.callback is not None:
                 self.callback(node.key, node.obj)
             del self.table[node.key]
 
@@ -149,7 +144,7 @@ class lrucache(object):
         node.obj = obj
 
         # Add the node to the dictionary under the new key.
-        self.table[node.key] = node
+        self.table[key] = node
 
         # We need to move the node to the head of the list. The node is the tail
         # node, so it directly preceeds the head node due to the list being
@@ -241,7 +236,7 @@ class lrucache(object):
         for i in range(n):
             node = self.head.prev
             if node.key is not None:
-                if self.callback:
+                if self.callback is not None:
                     self.callback(node.key, node.obj)
                 del self.table[node.key]
 
@@ -259,12 +254,11 @@ class lrucache(object):
         self.listSize -= n
 
 
-    # This method adjusts the doubly linked list so that 'node' directly preeceds
-    # the 'head' node. Note that it works even if 'node' already directly
-    # preceeds the 'head' node or if 'node' is the 'head' node, in either case
-    # the order of the list is unchanged.
+    # This method adjusts the ordering of the doubly linked list so that 'node'
+    # directly precedes the 'head' node. Because of the order of operations, if
+    # 'node' already directly precedes the 'head' node or if 'node' is the
+    # 'head' node the order of the list will be unchanged.
     def mtf(self, node):
-
         node.prev.next = node.next
         node.next.prev = node.prev
 
@@ -274,13 +268,17 @@ class lrucache(object):
         node.next.prev = node
         node.prev.next = node
 
+    # This method returns an iterator that iterates over the non-empty nodes in
+    # the doubly linked list in order from the most recently to the least
+    # recently used.
     def dli(self):
         node = self.head
-        for i in range(len(self)):
+        for i in range(len(self.table)):
             yield node
             node = node.next
 
 
+# The lruwrap class
 
 class lruwrap(object):
     def __init__(self, store, size, writeback=False):
@@ -288,13 +286,25 @@ class lruwrap(object):
         self.writeback = writeback
 
         if not self.writeback:
+            # Create a cache object. This cache will be used to (hopefully)
+            # speed up access to self.store.
             self.cache = lrucache(size)
         else:
+            # Create a set to hold the dirty keys. Initially empty to match the
+            # empty cache we are going to create.
             self.dirty = set()
+
+            # Define a callback function to be called by the cache when a
+            # key/value pair is about to be ejected. This callback will check to
+            # see if the key is in the dirty set. If so, then it will update
+            # the store object and remove the key from the dirty set.
             def callback(key, value):
                 if key in self.dirty:
                     self.store[key] = value
                     self.dirty.remove(key)
+                    
+            # Create the cache object. This cache will be used to (hopefully)
+            # speed up access to self.store. Set the callback function.
             self.cache = lrucache(size, callback)
 
     def __len__(self):
@@ -305,8 +315,9 @@ class lruwrap(object):
         assert self.writeback == False
         return len(self.store)
 
+    # Returns/sets the size of the managed cache.
     def size(self, size=None):
-        self.cache.size(size)
+        return self.cache.size(size)
 
     def clear(self):
         self.cache.clear()
@@ -316,26 +327,41 @@ class lruwrap(object):
 
     def __contains__(self, key):
         # XXX Should this bring the key/value into the cache?
+        # Check the cache first, since if it is there we can return quickly.
         if key in self.cache:
             return True
+
+        # Not in the cache. Might be in the underlying store.
         if key in self.store:
             return True
 
         return False
 
     def __getitem__(self, key):
+        # First we try the cache. If successful we just return the value. If not
+        # we catch KeyError and ignore it since that just means the key was not
+        # in the cache.
         try:
             return self.cache[key]
         except KeyError:
             pass
 
+        # It wasn't in the cache. Look it up in the store, add the entry to the
+        # cache, and return the value.
         value = self.store[key]
         self.cache[key] = value
         return value
 
     def __setitem__(self, key, value):
+        # Add the key/value pair to the cache.
         self.cache[key] = value
 
+        # If writeback is turned on then we just mark the key as dirty. That way
+        # when the key/value pair is ejected from the cache it will be written
+        # back to the store by the callback.
+        #
+        # If writeback is off (i.e. write-through is on) we go ahead and update
+        # the store as well.
         if self.writeback:
             self.dirty.add(key)
         else:
@@ -360,9 +386,14 @@ class lruwrap(object):
             if not found:  # If not found in cache or store, raise error.
                 raise KeyError
 
-        else:  # Write-through behavior, cache and store should be consistent
+        else:
+            # Write-through behavior cache and store should be consistent.
+            # Delete it from the store.
             del self.store[key]
             try:
+                # Ok, delete from the store was successful. It might also be in
+                # the cache, try and delete it. If not we catch the KeyError and
+                # ignore it.
                 del self.cache[key]
             except KeyError:
                 pass
@@ -383,8 +414,12 @@ class lruwrap(object):
                 yield key
 
     def values(self):
-        for key, value in self.items():
-            yield value
+        if self.writeback:
+            for key, value in self.items():
+                yield value
+        else:
+            for value in self.store.values():
+                yield value
 
     def items(self):
         if self.writeback:
@@ -399,11 +434,16 @@ class lruwrap(object):
             for item in self.store.items():
                 yield item
 
+
     def sync(self):
+        # A cache with write-through behavior is always in sync with the
+        # underlying store so we only need to work if write-back is on.
         if self.writeback:
+            # For each dirty key, peek at its value in the cache and update the
+            # store. Doesn't change the cache's order.
             for key in self.dirty:
-                value = self.cache.peek(key)  # Doesn't change the cache's order
-                self.store[key] = value
+                self.store[key] = self.cache.peek(key)
+            # There are no dirty keys now.
             self.dirty.clear()
 
     def flush(self):