go-faker / faker

Go (Golang) Fake Data Generator for Struct, previously https://github.com/bxcodec/faker
https://pkg.go.dev/github.com/go-faker/faker/v4
MIT License
614 stars 30 forks source link

Adding nested tags for slice of strings #49

Open HARDY8118 opened 4 months ago

HARDY8118 commented 4 months ago

As pointed out in the issue #37 (faker slice of urls don't work), struct fields do not support faker tags for the underlying field data type in case of slices. Taking a subsection of the mentioned issue

type Entity struct {
    ...
    Uris []string        `faker:"slice_len=3, url"`  // makes all structure empty. I just need slice with 3 urls
    ...
}

In the above code taken from issue #37 the Uris slice will not take into account the url tag.

For the mentioned issue, I suggest an extension to the faker struct tags syntax to allow nested tags with following syntax:

type SomeType struct {
    ...
    SomeSliceOfStrings []string `faker:"<tags for slice>, [<tags for string>]"`
    ...
}
// Example
type Emails struct {
    EmailList []string `faker:"slice_len=3, [email]"`
}

The PR is still a work in progress but I would like to hear more and discuss on the idea of using nested tags.

codecov-commenter commented 4 months ago

Codecov Report

Attention: Patch coverage is 76.92308% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 88.97%. Comparing base (6d0a05f) to head (7a75357).

Files Patch % Lines
faker.go 76.92% 4 Missing and 2 partials :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #49 +/- ## ========================================== - Coverage 89.01% 88.97% -0.05% ========================================== Files 13 13 Lines 1812 1832 +20 ========================================== + Hits 1613 1630 +17 - Misses 142 144 +2 - Partials 57 58 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bxcodec commented 4 months ago

Hi @HARDY8118

Thanks for the idea. I don't see any harm having the nested (yet). Maybe can you add more usecases/tests? Worried if we add this, then people will expecting more, want to see what are the limitation for this approach

And wondering from user experience, will this okay? As you can see from the issue, people expect it differently 🤔

HARDY8118 commented 4 months ago

Hi @bxcodec

As I mentioned initially in the PR, the idea is still a work in progress, I created this PR to get some initial thoughts on the overall idea of having such feature. As you can notice from the code change, currently I have implemented the changes just for slice of strings just to get some comments on the idea and have a discussion about it (while we are discussing on this PR I am drafting some more changes and adding tests for more use cases).

From user experience standpoint I think this is am okayish approach as it clearly defines what the user expects in both the contexts, first being from the context of slice (properties like slice_len) and other being from the context of the underlying data which the slice consists of (like in this case language and type of string). This will help moving forward if there exists some overlapping properties providing clarity and control over how each part behaves.

However I would love to hear from you and others as well on what can be done better for improving the user experience.

bxcodec commented 2 weeks ago

@rubemlrm any opinion from your side?