yegor256 / zache

Zero-footprint Ruby In-Memory Thread-Safe Cache: when a naive implementation is enough
https://www.yegor256.com/2019/02/05/zache.html
MIT License
41 stars 6 forks source link

Refactoring get method, add test for raise when block no given #14

Closed shindakioku closed 5 years ago

shindakioku commented 5 years ago

@yegor256

shindakioku commented 5 years ago

About raise "A block is required"

Do you really need the custom message?

You can remove this check and my test (add test for raise when block no given) will be succesffully execute but with some edit:

assert_equal(error.message, 'A block is required')

To

assert_equal(error.message, 'no block given (yield)')

yegor256 commented 5 years ago

@shindakioku looks good, see my comment above

yegor256 commented 5 years ago

@shindakioku I'm not sure it's more readable now: https://twitter.com/yegor256/status/1060771538448859136

aleksandr-kotlyar commented 5 years ago

@yegor256 Hi. What do you think about this kind of code?

def get(key, lifetime: 60 * 60) 
  raise 'A block is required' unless block_given?
  calc_lambda = -> { calc(key, lifetime) { yield } } 
  if @sync 
    return @mutex.synchronize { calc_lambda.call }
  end
  calc_lambda.call
end

Is it possible that this is a compromise solution between your understanding of the readable code and your reviewer?

yegor256 commented 5 years ago

@shindakioku I think the right solution would be to use DI for this coe