-
Notifications
You must be signed in to change notification settings - Fork 254
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add :finish_in_order option #339
Conversation
options[:finish_items] ||= [] | ||
options[:finish_items_done] ||= [] | ||
options[:finish_index] ||= 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can these be local variables ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can do this in the loop to not have the code twice ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
end
Let 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