Don't unnecessarily copy the key list

Author: Don Mitchell

bson/son.py

@@ -110,14 +110,20 @@ def copy(self):
     # efficient.
     # second level definitions support higher levels
     def __iter__(self):
-        for k in self.keys():
+        """
+        Cannot remove nor add keys while iterating
+        """
+        key_len = len(self.__keys)
+        for k in self.__keys:
+            if len(self.__keys) != key_len:
+                raise RuntimeError("son changed length during iteration")
             yield k
 
     def has_key(self, key):
-        return key in self.keys()
+        return key in self.__keys
 
     def __contains__(self, key):
-        return key in self.keys()
+        return key in self.__keys
 
     # third level takes advantage of second level definitions
     def iteritems(self):
@@ -139,8 +145,8 @@ def items(self):
         return [(key, self[key]) for key in self]
 
     def clear(self):
-        for key in self.keys():
-            del self[key]
+        self.__keys = []
+        super(SON, self).clear()
 
     def setdefault(self, key, default=None):
         try:
@@ -204,7 +210,7 @@ def __ne__(self, other):
         return not self == other
 
     def __len__(self):
-        return len(self.keys())
+        return len(self.__keys)
 
     def to_dict(self):
         """Convert a SON document to a normal Python dictionary instance.

doc/contributors.rst

@@ -53,9 +53,10 @@ The following is a list of people who have contributed to
 - Samuel Clay (samuelclay)
 - Ross Lawley (rozza)
 - Wouter Bolsterlee (wbolster)
-- Alex Grönholm (agronholm)
+- Alex Gr��nholm (agronholm)
 - Christoph Simon (kalanzun)
 - Chris Tompkinson (tompko)
 - Mike O'Brien (mpobrien)
 - T Dampier (dampier)
 - Michael Henson (hensom)
+- Don Mitchell (dmitchell)

test/test_son.py

@@ -111,5 +111,52 @@ def test_pickle_backwards_compatability(self):
         son_2_1_1 = pickle.loads(pickled_with_2_1_1)
         self.assertEqual(son_2_1_1, SON([]))
 
+    def test_iteration(self):
+        """
+        Test __iter__
+        """
+        # test success case
+        test_son = SON([(1, 100), (2, 200), (3, 300)])
+        for ele in test_son:
+            self.assertEqual(ele * 100, test_son[ele])
+        # test failure case
+        def break_iter():
+            for ele in test_son:
+                del test_son[ele]
+        self.assertRaises(RuntimeError, break_iter)
+
+    def test_contains_has(self):
+        """
+        has_key and __contains__
+        """
+        test_son = SON([(1, 100), (2, 200), (3, 300)])
+        self.assertIn(1, test_son)
+        self.assertTrue(2 in test_son, "in failed")
+        self.assertFalse(22 in test_son, "in succeeded when it shouldn't")
+        self.assertTrue(test_son.has_key(2), "has_key failed")
+        self.assertFalse(test_son.has_key(22), "has_key succeeded when it shouldn't")
+
+    def test_clears(self):
+        """
+        Test clear()
+        """
+        test_son = SON([(1, 100), (2, 200), (3, 300)])
+        test_son.clear()
+        self.assertNotIn(1, test_son)
+        self.assertEqual(0, len(test_son))
+        self.assertEqual(0, len(test_son.keys()))
+        self.assertEqual({}, test_son.to_dict())
+
+    def test_len(self):
+        """
+        Test len
+        """
+        test_son = SON()
+        self.assertEqual(0, len(test_son))
+        test_son = SON([(1, 100), (2, 200), (3, 300)])
+        self.assertEqual(3, len(test_son))
+        test_son.popitem()
+        self.assertEqual(2, len(test_son))
+
 if __name__ == "__main__":
     unittest.main()