I've written a module for repeating blocks of code, generally to cover issues related to eventual consistency and testing screen elements which may take some time to fully load. It looks like this:
class Repeater
def initialize(&block)
@repeat_block = block
end
def repeat(times: 25, delay: 0.2)
result = nil
times.times do
result = @repeat_block.call
break if @until_block.present? && @until_block.call
sleep(delay)
end
result
end
def until(&block)
@until_block = block
self
end
end
The Repeater takes two blocks of code, one to be run and the second to check for an exit criteria. Optionally the number of repeats and delays can be overwritten after the .repeat
method.
This is implemented in many places throughout the codebase, an example of such is:
result = false
Repeater.new do
result = event_present_in_database
end.until do
result
end.repeat(times: 10, delay: 0.1)
result
In this case the event_present_in_database
method is being repeated called until it returns true instead of false, which indicates that the event has been created as expected, or else times out indicating there's an issue.
This certainly works for what I want it to do, however it doesn't seem very neat and I'm sure it could be tidied up. I know Ruby has all sorts of tricks that I've yet to come across, is there a way to leverage these to make this nicer to use/easier to read.
1 Answer 1
There's not much weak points I can see.
The biggest one is the way to use it. One needs to declare a variable to be caught in the block (closure), and I think it's unnecessary. You should be able to do this:
Repeater.new do
event_present_in_database
end.until do |result|
result
end.repeat(times: 10, delay: 0.1)
If you changed the break
like like this:
break if @until_block.present? && @until_block.call(result)
The second, tiny point would be: make it clearer to understand the extraction to method conditionals:
# ...
break if exit_criteria_satisfied?(result)
# ...
private
def exit_criteria_satisfied?(result)
@until_block.present? && @until_block.call
end
That's all I got. The rest of the code seems pretty clear to me.
I know Ruby has all sorts of tricks
Yeah, but I don't know a single one that could help with the readability here.
-
1\$\begingroup\$ This is interesting, and is the sort of thing that I had in mind. One question, is there a way to do this if I need to interrogate the result of the Repeater? For example, if I'm waiting until there is a record present in the database, but once it's there I want to use the values within the record to do something else. My first thought is to store result as an instance variable, any better ways to do it? \$\endgroup\$Psycrow– Psycrow2019年11月19日 11:28:58 +00:00Commented Nov 19, 2019 at 11:28
-
\$\begingroup\$ Actually was just being a bit dense, you can assign the output of Repeater to a variable to have it accessible once it's finished running. Am going to experiment with this a bit and see how it handles real world applications \$\endgroup\$Psycrow– Psycrow2019年11月19日 14:24:14 +00:00Commented Nov 19, 2019 at 14:24
-
\$\begingroup\$ Yeah,
repeat
method still returns the lastresult
from the block, so you can assign it to any variable. \$\endgroup\$Greg– Greg2019年11月19日 14:36:52 +00:00Commented Nov 19, 2019 at 14:36
result
, and howevent_present_in_database
goes about doing its work. There could be other polling/retrying mechanisms you could use, such astry
-catch
, but I can't give you good advice based on this sketchy code. \$\endgroup\$