gh-106307: Add PyMapping_GetOptionalItem() by serhiy-storchaka · Pull Request #106308 · python/cpython (original) (raw)

I disagree. It will require explicitly setting result = NULL before every call of this function, and if you forgot to do this, it will crash your code in rare case of error, which may be not covered by tests. The risk of writing errorenous code is too high even if there would be a benefit from not setting the result to NULL.

Also, it makes the code simpler. Instead of saving the return code into a variable and checking it in two if\ s

int rc = PyMapping_GetOptionalItem(...); if (rc < 0) { goto error; } if (rc == 0) { // handle default }

you can get rid of that variable and insert the call directly in the if.

if (PyMapping_GetOptionalItem(...) < 0) { goto error; } if (value == NULL) { // handle default }

And there are many other cases in which it is convenient that you have two ways to check for result. I suggest you to make PyMapping_GetOptionalItem() not setting the result to NULL on error and to look how much code it will require to rewrite and how much uglier it will become. It is the same for PyObject_GetOptionalAttr.