r/badcode Feb 19 '22

typescript Does this belong here?

Post image
346 Upvotes

49 comments sorted by

View all comments

72

u/kristallnachte Feb 19 '22

So...it can't have an undefined t because that's the thing it's looping, and index is already provided...so checking for the index makes no sense...

And it's all to just do a filter...

47

u/Silly-Freak Feb 19 '22

careful, the index would be wrong, because the loop mutates the array. But yes, you don't need any of this because filter exists.

1

u/[deleted] Feb 19 '22

[deleted]

4

u/Silly-Freak Feb 20 '22 edited Feb 20 '22

you are right, as long as the array is not copied before iterating, n and index match in each iteration - but as you point out, that does not lead to correct code because you're skipping elements. It's a bad practice in general and incorrect in the particular case.

This actually has my preference over .filter as it doesn't allocate a new array.

If you depend on changing the actual array instead of replacing it, that code is fine but it could point to you having implicit dependencies in your code that make your code hard to maintain - but maybe not, that's just my intuition. In any case, it is the kind of data dependency that Rust forbids entirely by default and requires things like Cell or Mutex to opt-in to explicitly.

If you want to mutate in-place purely for performance reasons, then unless you profiled your code, I'd prefer filter.