Skip to content

Change data storage to files#461

Open
laraujo7 wants to merge 1 commit into
elixirs:masterfrom
laraujo7:data_storage
Open

Change data storage to files#461
laraujo7 wants to merge 1 commit into
elixirs:masterfrom
laraujo7:data_storage

Conversation

@laraujo7

Copy link
Copy Markdown
Contributor

I've added:

  • USAGE.md docs if applicable
  • CHANGELOG.md

@laraujo7

laraujo7 commented May 17, 2022

Copy link
Copy Markdown
Contributor Author

@vbrazo So, I wondered if we could move the data passed in the functions to a file, which would improve the reading of the modules.
This was my attempt, but I didn't know which type of file to use, so I left it as a txt. Do you think a yml or a json file would be better?

I would be down to change everything if it gets accepted 👍.

@vbrazo vbrazo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please provide benchmarks to show if this approach is faster than the current one?

I'd also like to get an opinion from @igas or @anthonator on this PR.

@laraujo7

laraujo7 commented May 17, 2022

Copy link
Copy Markdown
Contributor Author

So, I made some changes after running the benchmark as I found out that my solution was a bit slower. After my optimization, this was the result.

Screenshot 2022-05-17 at 17 55 44

What do you think @vbrazo ?

@laraujo7 laraujo7 changed the title Data storage Change data storage to file May 17, 2022
@laraujo7 laraujo7 changed the title Change data storage to file Change data storage to files May 17, 2022

@vbrazo vbrazo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@laraujo7 thanks for adding the benchmark.

@laraujo7
laraujo7 force-pushed the data_storage branch 3 times, most recently from e604590 to 2a9a44e Compare June 11, 2023 15:55
@laraujo7
laraujo7 force-pushed the data_storage branch 2 times, most recently from d7214bb to 24d0b82 Compare June 11, 2023 16:03
@nelsonmestevao

Copy link
Copy Markdown

This would make contributing to Faker much easier! 🙏

Comment thread lib/faker.ex
Comment on lines +173 to +176
count = Enum.count(data)

mapped_data =
data |> Enum.with_index() |> Enum.into(%{}, fn {k, v} -> {v, k} end) |> Macro.escape()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should call transform_into_map, right? @laraujo7

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