Skip to content
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

The array extension method :sample throw an argument error when the array is empty #94

Closed
pake007 opened this issue Nov 30, 2012 · 5 comments

Comments

@pake007
Copy link

pake007 commented Nov 30, 2012

Hi, stympy

I am using the newest version of faker, seems the extension of array method :sample breaks the default ruby :sample when the array is empty and there gives an argument.

  • ruby default :sample
    [].sample(3) => []
  • faker extension :sample
    [].sample(3) => wrong number of arguments (1 for 0)

Though this example seems a little wired, I really meet this error in my project. Please fix it.

@pake007
Copy link
Author

pake007 commented Nov 30, 2012

tip: I am using ruby 1.8.7

@pake007
Copy link
Author

pake007 commented Dec 3, 2012

Sorry, the first example is not ruby, should be rails

@michaelkrenz
Copy link

IMHO this is not really a bug. Ruby 1.8.7 does not have the Array#sample method (it was introduced in 1.9). The project's authors chose to implement a backport of that method, which was incomplete, but good enough for their purposes, as it was always used without arguments here. It relies on Ruby 1.8.7's Array#choice method, which does not accept an argument, hence the error message you mentioned. So in a Ruby 1.8.7 environment, you cannot simply use Array#sample, but should use the native Array#choice method or replicate that functionality using Kernel#rand.

On the other hand, as there is already a backport for this method in the project, I think it would be a worthwhile addition to improve it and make it fully compatible with its Ruby 1.9 counterpart. So I made a patch and wrote some corresponding tests. All tests ran successfully under Ruby 1.8.7 and 1.9.3.

The patch code is based on the excellent Backports project (https://github.com/marcandre/backports) - no need to reinvent the wheel here. I simplified it a bit to keep it manageable (I didn't want to replicate all of their Backport-internal features).

@pake007
Copy link
Author

pake007 commented Dec 3, 2012

Thanks @michaelkrenz, really rapid patch!
Btw, backports is excellent, starred it!

@antillas21
Copy link

@pake007 could you please close this issue? as @michaelkrenz pointed out his patch has been merged (years ago now).

@pake007 pake007 closed this as completed Sep 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants