Mark Dickinson <dicki...@gmail.com> added the comment: [Some of the Alexander's questions about procedures aren't really related to this issue; I've answered those offline. Here are the answers to the others.]
>> - initialize low to NULL, to match the Py_XDECREF(low) (could change >> that Py_XDECREF to Py_DECREF instead, but the code's more resistant >> to random refactorings this way --- see next item :) > > Good catch. Wouldn't the same argument apply to ilow? Wouldn't static code > checkers complain about redundant initialization? ilow doesn't need to be initialized because the PyArgs_ParseTuple is guaranteed to either fail or initialize it, and I can't see that the PyArgs_ParseTuple call is likely to change. But I admit that the lack of initialization here also makes me uncomfortable, especially in combination with the assert that's there. I might add an initialization. Do static code checkers really complain about redundant initializations? If anything, it seems like good defensive programming practice to initialize variables, even unnecessarily---later refactoring might make those initializations necessary. > >> - randomly refactor: regroup blocks for ease of reading > > I actually disagree that your regrouping makes the code clearer. In my > version, all idiosyncratic argument processing is done with borrowed > references first followed by common processing in three similar blocks. > This, however, is purely matter of taste. Note that I considered changing i* > names to raw* names, but decided not to introduce more changes than > necessary. In your grouping, however, the similarity of variable names is > more of an issue. This said, I don't really have any problem with your > choice. Okay, fair enough. I agree it's a matter of taste. I like the three separate blocks, one for each argument, especially since the refcounting semantics are clear: each block adds exactly one reference. But each to his own. :) > >> - don't do PyLong_FromLong(1) until it's needed ('zero' is different, >> since it's always used in the non-error case) > > Yes, I considered that. A further micro-optimization would be to initialize a > static variable in module initialization and reuse it in > get_len_of_range_longs as well. I decided to put it next to zero instead to > simplify the logic. Hmm. Possibly. I have an unhealthy and probably irrational aversion to non-constant static variables, even if the only time that the variable is changed is at module initialization. > >> - [micro-optimization]: don't pass a known zero value to >> get_range_long_argument > > Is it really worth it? Default start is probably not that common in case of > long arguments. Yes, possibly not. :) Partly I made this change because the assignment 'ilow = zero;' again raises a red flag for me, because it's not accompanied by a 'Py_INCREF(zero);' as I'd expect it to be. I realize that in this case it works out (because ilow is already a borrowed reference, and we're replacing it with a borrowed reference to zero), but it's sufficiently unusual that I have to think about it. This is personal preference again, I guess. ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue1533> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com