Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Enforce more structure on structs #77

Closed
dignifiedquire opened this issue Feb 23, 2018 · 4 comments
Closed

Enforce more structure on structs #77

dignifiedquire opened this issue Feb 23, 2018 · 4 comments

Comments

@dignifiedquire
Copy link
Contributor

I would like to propose to use something like https://github.com/go-playground/validator to allow simple validation of the content of structs, both on creation and on marshal/unmarshal.

Using the above package we could write something like this

type Block struct {
  RequiredField int `validation:"required"`
  OptionalField uint
}

func NewBlock(f1 int, f2 uint) (*Block, error) {
  out := &Block{
    RequiredField: f1,
    OptionalField: f2,
  }
  err := validate.Struct(out)
  if err != nil {
    return err
  }
  return out, nil
}
@dignifiedquire
Copy link
Contributor Author

Looking at go-ethereum they use https://godoc.org/github.com/fjl/gencodec to do something similar, but only for marshal and unmarshal operations.

@dignifiedquire
Copy link
Contributor Author

@phritz @whyrusleeping thoughts?

@phritz
Copy link
Contributor

phritz commented Mar 7, 2018

My reaction: this has the feeling of one of those things that seems obviously like a good at first blush (who wouldn't want valid objects?!) but that ends up encoding a rigidity that makes things hard to change. For example in tests always having to a have a fully formed Thingy even if you are only need part of it, or making it hard to change a struct once the system is launched.

BTW while it is a contentious topic, having required fields in your serialization format is generally considered harmful (eg https://github.com/google/protobuf/issues/2497). I've been bitten by it in various systems many times. I think I'd like to understand more about how we expect data and formats to change over time at various boundaries (our on-disk cache, in the network protocol if at all [what is the mechanism for adding a field?], actor & its memory, etc) before going too far down a path like this. Maybe you guys have a general strategy in your heads...

(Sorry had to delete the last comment because auto-linking caused our issue to show up on the linked issue)

@whyrusleeping
Copy link
Member

(@phritz btw, that autolinking is only visible by people who can access this repo, its scared me before but i verified it didnt show up when i wasnt logged in)

This seems interesting. But I agree with @phritz, this is a decelerating change, not an accelerating one. We can consider something like this after we have a running testnet.

@dignifiedquire dignifiedquire added this to the Testnet ⛺️ milestone Mar 15, 2018
@mishmosh mishmosh removed this from the Testnet ⛺️ milestone Mar 20, 2018
@dignifiedquire dignifiedquire changed the title Enforcing more structure on structs Enforce more structure on structs Sep 6, 2018
@anorth anorth closed this as completed Jul 22, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants