Samuel
@samuel.fm
spot the bug
![function getItemsWithNext() {
return items
.filter(item => {
if (isDeleted(item)) {
return false
}
return true
})
.map((item, i) => {
const next = items[i + 1]
return {
item,
next,
}
})
}](https://cdn.bsky.app/img/feed_thumbnail/plain/did:plc:p2cp5gopk7mgjegy6wadk3ep/bafkreifjayqiaf2nznful46gpc5kf2y5gjkktlmophvgefr2nvp7uorxtm "function getItemsWithNext() {
return items
.filter(item => {
if (isDeleted(item)) {
return false
}
return true
})
.map((item, i) => {
const next = items[i + 1]
return {
item,
next,
}
})
}")
18 1 32 18 1 32
David Buchanannot a bug, but `if (...) {return false}; return true;` is making my eye twitch
2 14Samuelthereโs a justifiable reason!
Samuel@samuel.fmยท 5 May 2024true, not the bug tho
I wrote it like that because I was simplifying this:
Bossettpassing functions back and forth always feels like there's so much syntax guff that you miss stuff like this - do we just not iterate through arrays anymore?
1 9
chelsea bridgehow harsh can i be here
2 9Samuelgo for it. break a leg.
1 3chelsea bridgekinda feel like the real bug is this pattern
1 7Samuelwhat part? we need a function that does 2 things
1. filter out deleted messages
2. provide the next message, for the UI to use
I'm curious what you'd do differently
2 2
Tibor MartiniI guess you could just use a gauge clause inside the map and drop the filter?
1 1Samuelwhat's a gauge clause? ๐
1 1Tibor MartiniSorry, I meant *guard clause ๐
1 1Samueloh right haha
so what, return null or something and then filter out afterwards?
1Tibor MartiniWould be an option, but wouldn't improve performance.
You could use reduce() instead: stackoverflow.com/questions/24...
Or use a normal loop.
2SamuelPerformance is not an issue here and reduce is wildly more unreadable and bug prone. Avoid reduce where possible imo unless you have a specific need
chelsea bridge> Or use a normal loop.
i was waiting for someone to say that
you don't have a function that needs to do two things, you have two distinct problems to solve and a stack of code that hides bugs and performance issues
1 8
Spoonful of bitterWhy not return each undeleted item with their next companion deleted or not?
The exception raised when the last item is not deleted is a feature, not a bug.
No specs, no bug.
Irish aka Drew (he/they)๐ณ๏ธโ๐When it comes to js, there is no limit
3
๐๐บ๐๐is it that you're still returning deleted items in next (and not supposed to)?
1 5Samuelish - the arrays are now different lengths, so `items[i + 1]` is not necessarily the next item
3 7
Biscuitmy brain hurts but wouldn't i + 1 eventually be an out of bounds rascal or is this some javascript wizardry
2 5
๐๐บ๐๐ah of course. dang so close to nailing the bluesky interview challenge
1 3
James MunnsAlso, does items[i + 1] index off the end of the array? does js have an equivalent to rust's "windows", where you get an iterator over like like (0, 1), (1, 2), ... (N-2, N-1), (N-1, N)?
Also is this just iterator invalidation issues?
1 1
Comfortably NumbBounds aren't meant to be broken.
4
fuckface "squeaky" from:meah, this is why you use the third argument to map functions, which passes the array being mapped - a correct line 9 could shadow it by changing the invocation to `.map((item, i, items) => {`
2 3Samuel
fuckface "squeaky" from:meone could also argue that `items` ought to come in as an argument, and whatever larger object provides an argumentless variant for a fixed `items` should be binding that function in its constructor
1 1Samuelin reality this was a class method, and items was a property
1 1fuckface "squeaky" from:meyeah, for all my griping about how JavaScript code ought to embrace lexical scoping more heavily, seeing it being spelled `this.items` probably would have made this bug a little more obvious to me
1 2fuckface "squeaky" from:meand of course this pattern should be followed for any reference into the source array for all iterative functions, precisely for reasons of composition when chained like this
Array - JavaScript | MDN
The Array object, as with arrays in other programming languages, enables storing a collection of multiple items under a single variable name, and has members for performing common array operations.
developer.mozilla.org
Christopher StevensThe i in the .map() may not quite line up in the out of scope items will itโฆ :) ๐
1 3SamuelYep ;)
Exhaustion Partyconst next considers the items array when it should be considering the filtered array, making next sometimes incorrect?
1 2
Michel Bestaat
SamuelNope! itโs JS, so itโs just undefined ;)
2 1Michel BestaatI guess the real bug is that the code to get the next accesses the original items array whereas the map function runs on a filtered array from which items may be removed.
Just don't have deleted items and all will be fine...
1 3Samuel
๐ฆ Plankie ๐ณ๏ธโ๐ Life, Uh, Finds a way ๐ฆThese kind of posts gives me butterflies in my tummy ๐ณ
3Michel BestaatI'd split it in two parts.
1. const filtered = items.filter ...
2. return filtered .map ....
2 1Samuelthatโd work
1Michel BestaatNot sure if there would be performance and/or additional memory costs though.
I probably cannot count on the compiler to optimize the code. :-)
1 1David BuchananThis was also my thought, although I'd call it something cool like nondeletedItems
1 2Michel BestaatWe really have to spell out everything for those bsky devs.
2Michel BestaatLovely
1
MetaflameIt's using indices of the filtered array on the original array, which returns irrelevant non-following items (as well as possibly deleted items). Also possible out-of-bounds access, but I think that's just `undefined`.
1
<-- GOOP | Spaxlines 4-7 could just be "return !isDeleted(item)" tbh
1 3Samuel<-- GOOP | Spaxahh I see
1
DontMakeMoiWhat's the effect of the comma after 'next'?
1 1
Justin BuistSyntactic sugar. Functionally, it does nothing, but if you add a 3rd item to the list by copy/pasting the 2nd one you don't have to fiddle with adding or removing commas by always leaving a trailing one.
1 1DontMakeMoiThanks! Thought that might be the case, but gawd I know so little JS.
A-livessecond last row?
greenlightening.bsky.socialThe real question is what unit tests did you write?
nilwould editing arrays while iterating through them be possible to warn/err with linting or compiler checks?