6 ms·

I'm not sure if that's something that he wrote or you wrote, but a `sum` function which has a side-effect of emptying its input list seems like a terrible thing

I'm not sure if that's something that he wrote or you wrote, but a `sum` function which has a side-effect of emptying its input list seems like a terrible thing. I hope you wouldn't use that in production.

jakeogh 11y ago
Why's that bad? If there's nothing else the input is needed for, why keep it around?
- lambda 11y ago
  When someone calls that, they wouldn't expect the input to be consumed. >>> list = [1,2,3] >>> sum(list)/len(list) >>> Traceback (most recent call last): File "<stdin>", line 1, in <module> ZeroDivisionError: integer division or modulo by zero
  - jakeogh 11y ago
    Thanks. I keep forgetting py isnt working on a copy of the var. copy.deepcopy() kinda fixes it, but then memory use is doubled, and the recursion limit kills it all anyway. I better go read my py code...
- cobaltblue 11y ago
  The only one who knows if the input is needed anymore is the caller, not all callers will agree, so it's bad form to let the called function decide automatically unless it's very clearly marked as a mutating, list-destroying function.
- adrusi 11y ago
  It's not bad necessarily to do it that way inline in another function, because there's no way to use it elsewhere without realizing that it has the side effect of emptying the list. However since this is recursive that's not realistically how it would be used. It wouldn't be bad if it were obvious what it did, which it isn't with the name "sum", but would be with the name 'sum_and_empty". For good measure, maybe "sum_in_linear_space_and_empty" is even better. It's also less efficient (although in the context of a recursive sum definition that's not too relevant). Essentially what you're doing here is using the length of the list as the iteration variable — the `i' in `for (i = n-1; i >= 0; i--)'. But `pop()' probably does more than decrement the length field, I imagine it also does some memory management bookkeeping at least. I also take issue with how `list' is mutated and accessed in the same expression. I don't really use python, but I think I recall reading that it in fact guarantees left-to-right evaluation of such expressions. Even still, it's confusing and makes programmers, especially those who write C/C++ where this is undefined behavior, uneasy.
- goalieca 11y ago
  because some idiot like me might join the team and call "sum" (or some parent that calls sum) thinking that everything is fine.
- sleepyams 11y ago
  Your function is fine but if you didn't want it to affect the original list you could do something like: def sum(lst): if lst: return lst[0] + sum(lst[1:]) return 0
  - jakeogh 11y ago
    Without knowing about the recursion limit that's what I should have written if I understood .pop() was mutating the input var's global state. It's a nice accident though.

jakeogh 11y ago

Why's that bad? If there's nothing else the input is needed for, why keep it around?

lambda 11y ago
When someone calls that, they wouldn't expect the input to be consumed. >>> list = [1,2,3] >>> sum(list)/len(list) >>> Traceback (most recent call last): File "<stdin>", line 1, in <module> ZeroDivisionError: integer division or modulo by zero
- jakeogh 11y ago
  Thanks. I keep forgetting py isnt working on a copy of the var. copy.deepcopy() kinda fixes it, but then memory use is doubled, and the recursion limit kills it all anyway. I better go read my py code...
cobaltblue 11y ago
The only one who knows if the input is needed anymore is the caller, not all callers will agree, so it's bad form to let the called function decide automatically unless it's very clearly marked as a mutating, list-destroying function.
adrusi 11y ago
It's not bad necessarily to do it that way inline in another function, because there's no way to use it elsewhere without realizing that it has the side effect of emptying the list. However since this is recursive that's not realistically how it would be used. It wouldn't be bad if it were obvious what it did, which it isn't with the name "sum", but would be with the name 'sum_and_empty". For good measure, maybe "sum_in_linear_space_and_empty" is even better. It's also less efficient (although in the context of a recursive sum definition that's not too relevant). Essentially what you're doing here is using the length of the list as the iteration variable — the `i' in `for (i = n-1; i >= 0; i--)'. But `pop()' probably does more than decrement the length field, I imagine it also does some memory management bookkeeping at least. I also take issue with how `list' is mutated and accessed in the same expression. I don't really use python, but I think I recall reading that it in fact guarantees left-to-right evaluation of such expressions. Even still, it's confusing and makes programmers, especially those who write C/C++ where this is undefined behavior, uneasy.
goalieca 11y ago
because some idiot like me might join the team and call "sum" (or some parent that calls sum) thinking that everything is fine.
sleepyams 11y ago
Your function is fine but if you didn't want it to affect the original list you could do something like: def sum(lst): if lst: return lst[0] + sum(lst[1:]) return 0
- jakeogh 11y ago
  Without knowing about the recursion limit that's what I should have written if I understood .pop() was mutating the input var's global state. It's a nice accident though.