schema icon indicating copy to clipboard operation
schema copied to clipboard

[feature] default values for scheme struct fields

Open zak905 opened this issue 2 years ago • 12 comments

Is your feature request related to a problem? Please describe.

in case values are not set for a non required value, it would be nice to have the possibility to set a default value using a tag, for example schemaDefault or just default

Describe the solution you'd like

type Person struct {
    Name  string `schema:"name,required"`
    Phone string `schema:"phone" schemaDefault:"+1234567890"`
}

update (12-03-2022)

the design has been slightly changed to use a tag option rather than a standalone tag

type Person struct {
    Name  string `schema:"name,required"`
    Phone string `schema:"phone,default=+1234567890"`
}

zak905 avatar Jul 18 '21 15:07 zak905

Here a fist attempt: https://github.com/zak905/schema/commit/66f64fe87c96b3d214096829e63dfbc1e4340e20

I made the following assumptions:

  • only primitive/basic types can have default values (and their pointers as well)
  • required fields having a default value will result in an error
  • default tag is used to set default value for a schema. It could also be a tag option instead of new tag (e.g schema:"phone,default=+12334555"). I am leaning in favor of new tag

Let me know if have any comments and if you would be interested in adding this to upstream.

zak905 avatar Jul 30 '21 14:07 zak905

I added some improvements. Now, the default values are set for nested structs (or pointer to struct) fields: https://github.com/zak905/schema/commit/36c9802c2068b86e8babaa00525900653590754c

zak905 avatar Aug 01 '21 15:08 zak905

PR submitted. Looking forward to hear the community's feedback

zak905 avatar Aug 08 '21 18:08 zak905

This issue has been automatically marked as stale because it hasn't seen a recent update. It'll be automatically closed in a few days.

stale[bot] avatar Jan 09 '22 02:01 stale[bot]

hey stale bot, please remove the stale tag

zak905 avatar Jan 09 '22 13:01 zak905

Really interesting feature. Two things I'm curious about:

  • what are some use cases you considered for this feature? For decoding from HTML-based POSTs, a client can readily provide default values for a form input (e.g., <input type="text" name="firstName" value="David">). For encoding, I can see how this provides an extra convenience, but just as well could see how a constructor (e.g., func NewPerson() Person) would be a preferable path in a codebase.
  • what led to introducing a new struct tag? I see you considered parsing the tag read by this package (schema), but opted for adding default. Were there any limitations in the codebase preventing doing, e.g., schema:"phone,default=+12334555"?

DavidLarsKetch avatar Jan 17 '22 00:01 DavidLarsKetch

Hi @DavidLarsKetch,

Thanks for the feedback.

what are some use cases you considered for this feature? For decoding from HTML-based POSTs, a client can readily provide default values for a form input (e.g., ). For encoding, I can see how this provides an extra convenience, but just as well could see how a constructor (e.g., func NewPerson() Person) would be a preferable path in a codebase.

schema is meant for form values, but can be also be used to parse a request's query parameters as a struct.

decoder.Decode(params, r.URL.Query())

Either case, this PR aims to make decoding more convenient. Yes it's possible to provide values in forms, but in case the process is to be handled backend side. It think it makes it not only simpler, but also self-explanatory. You can just checkout the struct and see which values has defaults. Instead of having to if-else every struct type to check and set default values, you could have for example a generic parameters struct of type interface{} that you decode in a middleware and put in the context. You could then cast to your struct, for example, based on the endpoint.

what led to introducing a new struct tag? I see you considered parsing the tag read by this package (schema), but opted for adding default. Were there any limitations in the codebase preventing doing, e.g., schema:"phone,default=+12334555"?

If I remember well, there are at least two reasons. The first is the additional parsing, and the second is the order of options. The current tag options parsing puts all options in an array in the order they are provided, and therefore you have to loop a second time to find the default option. So I guess a tag requires less work, but I don't mind changing if the option alternative is more acceptable semantically.

zak905 avatar Jan 19 '22 13:01 zak905

I was following with the use case pretty well and agree, prima facie, that there's a clarity to signalling a default value, but got a bit hung up on this line:

you could have for example a generic parameters struct of type interface{} that you decode in a middleware and put in the context. You could then cast to your struct, for example, based on the endpoint. I'm unclear on the need for a middleware here, or, is this just an alternative pattern?

With regards to the struct tag, from my experience, I have not used a package exposing more than 1 struct tag and instead am familiar with packages that expose 1 struct tag and use a separator like , or ; (e.g., json:foobar,omitempty). For example, while unlikely, if someone were to use this package alongside schema (with your changes in #183) to supply filling in default values, the struct tags would collide.

Looking at #183 more specifically, I noticed you omit supporting default values for structs, pointers, and slices. The first two I have a hard time envisioning how that would work, but a default slice of strings, for example, seems less of a challenge.

DavidLarsKetch avatar Jan 20 '22 13:01 DavidLarsKetch

I'm unclear on the need for a middleware here, or, is this just an alternative pattern?

No there is no necessity for middleware, it just happens that this is the way I am using it in some projects, so I was just giving an example.

It should not be complicated changing from a tag to a tag option, I admit I was a bit lazy on this one:) `schema:"phone,default=+12334555" sounds like more golang style indeed.

If we are to introduce slices, then I guess we have to support also slices for all primitive types, not only strings. How would we go about delimiters? I think a comma , would be confusing since comma is used to separate options : schema:"list,default=1,2,4"

zak905 avatar Jan 23 '22 16:01 zak905

I create my own defaultvalue:"value" tag and before bind I set that default value tag in the url. Values with reflect. It's look ugly, but It works pretty well.

aigoya avatar Jan 31 '22 09:01 aigoya

cool, so we all agree that a tag option is less cumbersome than a standalone tag

zak905 avatar Jan 31 '22 10:01 zak905

Hi @DavidLarsKetch,

as per our latest discussion, I made the following changes:

  • changing from a default tag to a default option, e.g schema:"b,default:15"
  • adding support for slices for all the primitive types. I used | as the separator, because , does not work and is interpreted by go as providing an additional option
  • making the decoding more error / panic proof by handling cases where incompatible default value is provided for a certain type, e.g if you provide a string as a default value for an int type

Once we finalize the above, I can update the README

zak905 avatar Mar 12 '22 20:03 zak905

Done in #183 and #209

zak905 avatar Mar 07 '24 12:03 zak905