[Zodb-checkins] SVN: ZODB/branches/alienoid-btrees_setdefault/src/BTrees/ BTree_setdefault(): Rewrote to look more like Python's

[Tim Peters]

Log message for revision 38146:
  BTree_setdefault():  Rewrote to look more like Python's
  dict.setdefault implementation, and to use 4-space indents
  like the rest of the file.  This fixes some endcase bugs too.
  
  Alas, treating the default "key missing value" of None
  correctly reveals that this isn't a sane policy for
  II, OI, or IF BTrees:  they can't have None as a value.
  
  testSetdefault():  Rewrote to reflect correct treatment of
  a missing default, and added an XXX noting that the repaired
  test fails now for II, OI, and IF trees.
  
  Note:  Buckets need to grow a setdefault() implementation
  too.
  

Changed:
  U   ZODB/branches/alienoid-btrees_setdefault/src/BTrees/BTreeTemplate.c
  U   ZODB/branches/alienoid-btrees_setdefault/src/BTrees/tests/testBTrees.py

-=-
Modified: ZODB/branches/alienoid-btrees_setdefault/src/BTrees/BTreeTemplate.c
===================================================================
--- ZODB/branches/alienoid-btrees_setdefault/src/BTrees/BTreeTemplate.c	2005-08-29 17:18:41 UTC (rev 38145)
+++ ZODB/branches/alienoid-btrees_setdefault/src/BTrees/BTreeTemplate.c	2005-08-29 19:29:27 UTC (rev 38146)
@@ -1703,23 +1703,30 @@
 static PyObject *
 BTree_setdefault(BTree *self, PyObject *args)
 {
-        PyObject *key, *d = Py_None, *r;
+    PyObject *key;
+    PyObject *failobj = Py_None;  /* default */
+    PyObject *value;  /* return value */
 
-        UNLESS (PyArg_ParseTuple(args, "O|O", &key, &d)) return NULL;
-        if ((r = _BTree_get(self, key, 0)))
-                return r;
-        UNLESS (PyErr_ExceptionMatches(PyExc_KeyError)) return NULL;
-        PyErr_Clear();
+    if (! PyArg_UnpackTuple(args, "setdefault", 1, 2, &key, &failobj))
+    	return NULL;
 
-        Py_INCREF(d);
-        if (d == Py_None)
-                return d;
+    value = _BTree_get(self, key, 0);
+    if (value != NULL)
+        return value;
 
-        if (_BTree_set(self, key, d, 0, 0) < 0) {
-                Py_DECREF(d);
-                return NULL;
-        }
-        return d;
+    /* The key isn't in the tree.  If that's not due to a KeyError exception,
+     * pass back the unexpected exception.
+     */
+    if (! PyErr_ExceptionMatches(PyExc_KeyError))
+        return NULL;
+    PyErr_Clear();
+
+    /* Associate `key` with `failobj` in the tree, and return `failobj`. */
+    value = failobj;
+    if (_BTree_set(self, key, failobj, 0, 0) < 0)
+        value = NULL;
+    Py_XINCREF(value);
+    return value;
 }
 
 

Modified: ZODB/branches/alienoid-btrees_setdefault/src/BTrees/tests/testBTrees.py
===================================================================
--- ZODB/branches/alienoid-btrees_setdefault/src/BTrees/tests/testBTrees.py	2005-08-29 17:18:41 UTC (rev 38145)
+++ ZODB/branches/alienoid-btrees_setdefault/src/BTrees/tests/testBTrees.py	2005-08-29 19:29:27 UTC (rev 38146)
@@ -1092,12 +1092,21 @@
 
     def testSetdefault(self):
         t = self.t
+
+        # XXX This test fails for II, OI, and IF trees:  they can't have
+        # XXX None as a value.
         self.assert_(t.setdefault(1) is None)
-        self.assertEqual(t.setdefault(1, 5), 5)
-        self.assert_(t[1] == 5)
-        self.assertEqual(t.setdefault(1, 3), 5)
+        # That should also have associated 1 with None in the tree.
+        self.assert_(t[1] is None)
+        # And trying to change it again should have no effect.
+        self.assert_(t.setdefault(1, 666) is None)
 
+        # Same thing, but with an explicit default.
+        self.assertEqual(t.setdefault(2, 5), 5)
+        self.assertEqual(t[2], 5)
+        self.assertEqual(t.setdefault(2, 666), 5)
 
+
 # tests of various type errors
 
 class TypeTest(TestCase):