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

Fix Faker::Vehicle Array Access #705

Closed
wants to merge 1 commit into from
Closed

Fix Faker::Vehicle Array Access #705

wants to merge 1 commit into from

Conversation

rui-ribeiro
Copy link

@rui-ribeiro rui-ribeiro commented Sep 21, 2016

Just a little fix accessing the array.

I don´t know if this is the best way to fixed, feel free to comment.

Fixs #627

@rui-ribeiro rui-ribeiro changed the title Fix Array Access Fix Faker::Vehicle Array Access Sep 21, 2016
@rui-ribeiro
Copy link
Author

See PR #644

@SagareGanesh
Copy link
Contributor

SagareGanesh commented Sep 21, 2016

@rui-ribeiro Your PR is right. But test cases related to Vehicle are faild. The reason behind it is, when you access this

manufacture = fetch_all('vehicle.manufacture').sample

In your app, it returns hash whose keys are symbols. And when you run test cases it returns hash whose keys are strings. Strange behavior.

@rui-ribeiro
Copy link
Author

Yes, exactly that. I don´t know why it behaves like that, must me some require or extension.

That´s why i referenced PR #644 because of the conversion of the hash for indifferent hash.

@SagareGanesh
Copy link
Contributor

SagareGanesh commented Sep 21, 2016

Another solution for this problem is, You can use or( || ) in your code.
For Example: In vin method you can write like

vehicle_identification_number = (manufacture['wmi'] || manufacture[:wmi]).split('').concat( Array.new(14) { c.sample } )

It will solve both problems. thanks

stympy added a commit that referenced this pull request Jan 3, 2017
@stympy stympy closed this Jan 3, 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

Successfully merging this pull request may close these issues.

3 participants