[Webkit-unassigned] [Bug 67823] [NRWT] Provides a simple LRU cache class in Python.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=67823





--- Comment #5 from Hayato Ito <hayato at chromium.org>  2011-09-14 22:04:14 PST ---
(From update of attachment 107322)
Thank you! The patch looks good!
I think there are still some corner cases we must handle carefully.

View in context: https://bugs.webkit.org/attachment.cgi?id=107322&action=review

> Tools/Scripts/webkitpy/common/lru_cache.py:39
> +        size: a cache memory size

'capacity' might be a better name than 'size' in this context.

> Tools/Scripts/webkitpy/common/lru_cache.py:42
> +        assert size > 0, "You should set the LRUCache size to not less than zero."

You might want to include the actual value of |capacity| in the assertion message.
like:
  assert capacity > 0, "capacity (%s) must be greater than zero." % capacity

> Tools/Scripts/webkitpy/common/lru_cache.py:43
> +        self.first = None

if you don't want to expose |first| and |last|, that should be named like |_first| and |_last|.

> Tools/Scripts/webkitpy/common/lru_cache.py:49
> +        if self.first == None:

I think 
  if not self.first:
is more pythonic than
  if self.first == None:

At least it should be 'self.first is None' instead of 'self.first == None'.

> Tools/Scripts/webkitpy/common/lru_cache.py:65
> +        node = Node(key, value, self.first)

When _one_node is called, self.first is always None, isn't it?
Therefore this should be:
  node = Node(key, value, None)

> Tools/Scripts/webkitpy/common/lru_cache.py:71
> +        if self.first.key == key:

If self.first is None, this will fail.

> Tools/Scripts/webkitpy/common/lru_cache.py:99
> +            self.first = node.prev

If LRUCache contains one item, (k, v) and del[k] is called, it seems self.first will become 'None' here. So the following self.first.next will fail. Could you add a testcase for this?

And if self.first and self.last point to the same node, it seems we also have to set self.last = None here.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.