Conversation
| options[:finish_items] ||= [] | ||
| options[:finish_items_done] ||= [] | ||
| options[:finish_index] ||= 0 |
There was a problem hiding this comment.
You can't use local variables, as they need to maintain the state between invocations.
As they are being called via a class method, it can't use instance variables either.
The options are dup'ed so works even when invoked multiple times with the same options
| options[:finish].call(item, index, result) | ||
| options[:finish_index] += 1 |
There was a problem hiding this comment.
can do this in the loop to not have the code twice ?
There was a problem hiding this comment.
To do it in the loop, you would have to store the item first, e.g. something like this:
This is a bit more readable, but uses a tiny bit more memory.
def instrument_finish_in_order(item, index, result, options)
options[:mutex].synchronize do
options[:finish_items] ||= []
options[:finish_items_done] ||= []
options[:finish_index] ||= 0
options[:finish_items][index] = item
options[:finish_items_done][index] = true
break unless index == options[:finish_index]
index.upto(options[:finish_items].size).each do |old_index|
break unless options[:finish_items_done][old_index]
old_item = options[:finish_items][old_index]
options[:finish].call(old_item, old_index, result)
options[:finish_index] += 1
end
end
endLet me know if you prefer this version and I'll update the PR
|
FYI refactored #340 |
|
lmk if you can spot any more issues, otherwise I'll merge this and then release a new minor 🎉 |
|
1.24.0 |
See #338 for discussion