Mark Dickinson <[email protected]> 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 <[email protected]>
<http://bugs.python.org/issue1533>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
