Cleanup stack #16326
Cleanup stack #16326
Conversation
| self.push(thiso) | ||
| for _ in bubbles: | ||
| self.push(elem) | ||
| for _ in top_elements: | ||
| self.push(o) | ||
| return o |
There was a problem hiding this comment.
Shorter would be something like
count = self._elements.count(o)
if not count: ...
self._elements = [e for e in self._elements if e != o] + n * [o]
no?
Edit: well, perhaps not, _pos would also need to be adjusted.
There was a problem hiding this comment.
That's what I meant with "should be tested". I believe that there are subtle differences if you have objects that are equal but not identical. Not that I think the sematics for that is considered in the design and well defined, but I don't want to change this without really thinking about it.
Not going to do this right now as part of this PR.
|
Fundamentally I agree, but strictly requiring test coverage will reduce changes to the code (e.g. I wouldn't have cleaned this up, if I would have to write test code first). There's a trade-off between the benefit of code cleanup and the risk of breaking something. Therefore, I'v restricted this PR to very basic changes, detailed in the first commit. I think they are easy enough to follow, so that the risk of breaking something is small enough. |
PR Summary
self._elementsis a list. The canoncial non-empty check isif not self._emenentsrather thatnif not len(self._elements).list.copy()instead oflist[:](available since Python 3.3).old->old_elementsbubbles->top_elementsthiso->elemNote:
bubble()andclear()can be implemented much more efficiently. But that's for another PR because I'd also have to write tests for that.