Delegate :sample to `records`

active-hash / active_hash

A readonly ActiveRecord-esque base class that lets you use a hash, a Yaml file or a custom file as the datasource

MIT License

1.21k stars 178 forks source link

Delegate :sample to `records` #189

Closed ekrc-1 closed 4 years ago

ekrc-1 commented 4 years ago

In this PR, delegate macro will accept #sample method to records (same as ActiveRecord)

before

[1] pry(main)> Foo.all.sample
NoMethodError: undefined method `sample' for #<ActiveHash::Relation:0x000055874716f4f8>

[2] pry(main)> Foo.all.to_a.sample
=> #<Foo:0x000055873fe07920 @attributes={:id=>3, :type=>"BAZ"}>

after

[1] pry(main)> Foo.all.sample
=> #<Foo:0x000055b80956ca40 @attributes={:id=>2, :type=>"BAR"}>

syguer commented 4 years ago

@1grc Thank you for your contribution 😄 Could you write some test codes?

ekrc-1 commented 4 years ago

@1grc Thank you for your contribution 😄 Could you write some test codes?

@syguer OK, added test code.
I worried if I should add a test code for 'return a random element or n random elements'.
Is this in line with your wishes?

syguer commented 4 years ago

@1grc Thanks! Yes, it looks good!

[IMO] It seems the first test is covered by the second one so I think it is enough by second case.

ekrc-1 commented 4 years ago

@syguer Thank you for your kindness! 😄

[IMO] It seems the first test is covered by the second one so I think it is enough by second case.

Right. Do I need to delete the first test case? (Is it unnecessary because it has already been merged?)

syguer commented 4 years ago

No, you don't have to do. Thanks 👍