-
-
Notifications
You must be signed in to change notification settings - Fork 49
Description
I noticed this on a situation where I had the current progress, but not the exact difference between the current value and the last value I had. In my case it wasn't actually bad since it's just a simple count, but it feels wrong, and I can already see some situations where this could be a problem, the most obvious being asynchronous updates, a good example that comes to mind would be "how much data has already been received", but in my case I try to make my programs use STDIN/STDOUT as much as I can (Quite useful when you use grep, awk, wc, tail, etc.).
It'd be a simple update tolib/progress_bar.rb, a new function
def update!(count = 1)
self.count = count
now = ::Time.now
# last_write uses @, count uses self, it'd be nice to pick and use only one
# also, you used || which means if either last_write is > 2 or @count is ABOVE max, it should write.
if (now - @last_write) > 0.2 || self.count >= max
write
@last_write = now
end
endThough I'd suggest refactoring some stuff, it's nice do consider it now since code is still quite simple, if it grows like this there'll be a lot of duplication.
Mostly it'd be a pain when adding stuff like different possible write conditions, different triggers for change, different output options/styles, and probably more. Also this is just an example, what matters is separation of concerns and each function doing only one thing. Plus, I didn't normalize but you should really decide on either @ or self or anything.
# increment and any functions that may appear (decrement?) should also be like this
def update!(count = 1)
self.count = count
do_write
end
#On private
# if multiple branches, could either take arguments or use self./@
def should_write? now
slow_enough = (now - @last_write) > 0.2
over_max = self.count >= max
return true if slow_enough || over_max
return false
end
def do_write
now = ::Time.now
return nil unless should_write? now # if !should_write also seems good.
@last_write = time
write
end