11 ms·
>The Valid method takes a context (which is optional but has been useful for me in the past) and returns a map. If there is a problem with a field, its name is
by mtlynch 3y ago
>The Valid method takes a context (which is optional but has been useful for me in the past) and returns a map. If there is a problem with a field, its name is used as the key, and a human-readable explanation of the issue is set as the value.
I used to do this, but ever since reading Lexi Lambda's "Parse, Don't Validate," [0] I've found validators to be much more error-prone than leveraging Go's built-in type checker.
For example, imagine you wanted to defend against the user picking an illegal username. Like you want to make sure the user can't ever specify a username with angle brackets in it.
With the Validator approach, you have to remember to call the validator on 100% of code paths where the username value comes from an untrusted source.
Instead of using a validator, you can do this:
type Username struct {
value string
}
func NewUsername(username string) (Username, error) {
// Validate the username adheres to our schema.
...
return Username{username}
}
That guarantees that you can never forget to validate the username through any codepath. If you have a Username object, you know that it was validated because there was no other way to create the object.
[0] https://lexi-lambda.github.io/blog/2019/11/05/parse-don-t-validate/ https://lexi-lambda.github.io/blog/2019/11/05/parse-don-t-va...
- oorza 3y agoConceptually equivalent to the ancient arts of private constructors and factory methods.
- Cthulhu_ 3y agoWhich (in Java) were then abstracted away in... interesting annotations.
- xboxnolifes 3y agoCrazy that actually using your type system leads to better code. Stop passing everything around as `string`. Parse them, and type them.
- stickfigure 3y agoThere's a name for this anti-pattern: "Stringly typed"
- timcobb 3y agoBash :(
- tdeck 3y agoThis term is typically used to refer to things like data structures and numerical values all being passed as strings. I don't think a reasonable person would consider storing a username in a string to be "stringly typed".
- samatman 3y agoThe One True Wiki[0] says "Used to describe an implementation that needlessly relies on strings when programmer & refactor friendly options are available." Which is exactly what's going on here. A username has a string as a payload, but that payload has restrictions (not every string will do) and methods which expect a username should get a username, not any old string. [0]: https://wiki.c2.com/?StringlyTyped https://wiki.c2.com/?StringlyTyped
- tdeck 3y agoI don't agree that this example is more "programmer friendly". Anything you want to do with the username other than null check and passing an argument is going to be based directly on the string representation. Insert into a database? String. Display in a UI? String. Compare? String comparison. Sort? String sort. Is it really more "programmer friendly" to create wrapper types for individual strings all over your codebase that need to have passthrough methods for all the common string methods? One could argue that it's worth the tradeoff but this C2 definition is far from helpful in setting a clear boundary. Meanwhile the real world usages of this term I've seen in the past have all been things like enums as strings, lists as strings, numbers as strings, etc... Not arbitrary textual inputs from the user.
- asp_hornet 3y agoNow what? the username is in an unexported field and unusable? I can kind of see what its going for but it seems like a way just to add another layer of wrapping and indirection.
- pants2 3y agoIt would need a getter here. Probably good to keep it immutable, if you want guarantees that it will never be changed to something that violates the username rules.
- asp_hornet 3y ago> need a getter Yeah, thats what I figured. Im not sure if I want the tradeoff of calling .GetValue in multiple places just to save calling validate in maybe 2 or 3 places. Not to mention I cant easily marshal/unmarshal into it and next week valid username is a username that doesnt already exist in the database. Maybe this approach appeals to people and Im hesitant to say “that’s not how Go is supposed to be written” but for me this feels like “clever over clear”.
- klodolph 3y ago> Yeah, thats what I figured. Im not sure if I want the tradeoff of calling .GetValue in multiple places just to save calling validate in maybe 2 or 3 places. The tradeoff is not that you save calling validate, it’s that you avoid forgetting to call validate in the first place, because when you forget to validate, you get a type error. IMO it’s a little more clear this way: type Ticket struct { requestor Username assignee Username } It lets you write code that is little more obvious.
- asp_hornet 3y agoI’m not sure I understand. In your example you’ve grouped related data in a struct and validating that it matches your system’s invariants, that feels good to me. The original example was more “wrap a simple type in an object so it’s always validated when set” which looks beautiful when you don’t have the needed getters in the example nor show all the Get call sites opposed to the 1 or 2 New call sites. All in the name of “we don’t want to set the username without validation” but without private constructors Username{“invalid”} can be invoked, the validation circumvented and I’m not convinced the overhead we paid was worth it.
- cogman10 3y agoThe issue is DRY often comes to wreck this sort of thing. Some devs will see "Hmm, Username is exactly the same as just a string so let's just use a string as Username is just added complexity". I've tried it with constructs like `Data` and `ValidatedData` and it definitely works, but you do end up with duplicate fields between the two objects or worse an ever growing inheritance tree and fields unrelated to either object shared by both. For example, consider data looking like Data { value string } and ValidatedData looking like ValidatedData { value int } There's a mighty temptation for some devs to want to apply DRY and zip these two things together. Unfortunately, that can really be messy on these sorts of type changes and the where of where validation needs to happen gets muddled.
- xboxnolifes 3y agoExcept Username is not exactly the same as string, and that's important. Username is a subset of string. If they were equivalent, we wouldn't need to parse/validate. The often misinterpreted part of DRY is conflating "these are the same words, so they are the same", with "these are the same concept, so they are the same". A Username and a String are conceptually different.
- cogman10 3y agoDRY is just "Do not repeat yourself". And a LOT of devs take that literally. It's not "Do not repeat concepts" (which is what it SHOULD be but DRC isn't a fun acronym). Unfortunately "This is the same character string" is all a DRY purist needs to start messing up the code base. I honestly believe that "DRY" is an anti-pattern because of how often I see this exact behavior trotted out or espoused. It's a cargo cult thing to some devs.
- oorza 3y agoThat's why I like to tell people to always remember to stay MOIST - the Most Optimal is Implicitly the Simplest Thing. When you add complexity to DRY out your code, you're adding a readability regression. DRY matters in very few context beyond readability, and simplicity and low cognitive load need to be in charge. Everything else you do code-style wise should be in service of those two things.
- skybrian 3y agoThis is a good design pattern, but be wary of doing validation too early. The design pattern allows you to do it as early or late as you like, but doesn't tell you when to do it. Often it's best to do it as part of parsing/validating some larger object. See Steven Witten's "I is for Intent" [1] for some ideas about the use of unvalidated data in a UI context. [1] https://acko.net/blog/i-is-for-intent/ https://acko.net/blog/i-is-for-intent/
- lolinder 3y agoI read through that piece and strongly disagree with the premise that their insight is somehow at odds with leaning into the type system for correctness. The legitimate insight that they have is that anchoring the state as close as possible to the user input is valuable—I think that that is a great insight with a lot of good applications. However, there's nothing that says you can't take that user-centric state and put it in a strongly typed data structure as soon as possible, with a set of clearly defined and well-typed transitions mapping the user-centric state to the derived states. Edit: looks like there was discussion on this the other day, with a number of people making similar observations—https://news.ycombinator.com/item?id=39269886 https://news.ycombinator.com/item?id=39269886
- skybrian 3y agoA text file and an abstract syntax tree can both be rigorously represented using types, but one is before parsing and other is after parsing. The question is which one is more suitable for editing? Text has more possible states than the equivalent AST, many of which are useful when you haven't typed in all the code yet. Incomplete code usually doesn't parse. This suggests that drafts should be represented as text, not an AST. And maybe similarly for drafts of other things? Drafts will have some representation that follows some rules, but maybe they shouldn't have to follow all the rules. You may still want to save drafts and collaborate on them even though they break some rules. In a system that's not an editor, though, maybe it makes sense to validate early. For a command-line utility, the editor is external, provided by the environment (a shell or the editor for a shell script) so you don't need to be concerned with that.
- deleted 3y ago[deleted]
- andrus 3y agoI’ve found it hard to apply this pattern in Go since, if Username is embedded in a struct, and you forget to set it, you’ll get Username’s zero value, which may violate your constraints.
- mrklol 3y agoWhy? You can easily call NewUsername inside NewAccount for example, just return the error. Or did I misunderstood?
- BreakfastB0b 3y agoBecause go doesn’t have exhaustiveness checking when initialising structs. Instead it encourages “make the zero value meaningful” which is not always possible nor desirable. I usually use a linter to catch this kind of problem https://github.com/GaijinEntertainment/go-exhaustruct https://github.com/GaijinEntertainment/go-exhaustruct
- vorticalbox 3y agoI like this but in the examples would volume be calculated by width/length rather than being set?
- Cthulhu_ 3y agoBut if you then create a constructor / factory method for that struct, not setting it would trigger an error. But this is one of the problem with Go and other languages that have nil or no "you have to set this" built into their type system: it relies on people's self-discipline, checked by the author, reviewer, and unit test, and ensuring there's not a problem like you describe takes up a lot of diligence.
- randomdata 3y agoIt only relies on unit tests. The people can fail all day long and the unit tests will catch it every single time. Not special unit tests that attempt to seek out such issues, the same unit tests you are writing in languages that have a stricter type system. If you forget to initialize a field and the tests don't notice, you didn't need the field in the first place, so it won't matter if it is left in an invalid state. You just don't get the squiggly lines in your text editor. That's the tradeoff.
- 3pm 3y agoAKA 'Value Object' from DDD or a similar 'Quantity' accounting pattern. Another angle is that this fixes 'Primitive Obsession' code smell.
- costco 3y agoJust do type Username string And replace return Username{username} with return Username(username)
- daveFNbuck 3y agoIf you do that, people outside the package can also do Username(x) conversions instead of calling NewUsername. Making value package private means that you can only set it from outside the package using provided functionality.
- mtlynch 3y agoThe problem there is that you lose the guarantee that the parser validated the string value. A caller can just say: // This is returning an error for some reason, so let's do it directly. // username, err := parsers.NewUsername(raw) username := parsers.Username(raw) You also get implicit conversions in ways you probably don't want: var u Username u = "<hello>" // Implicitly converts from string to Username
- costco 3y agoThat's true I did not think of that.
- Scarblac 3y ago> and a human-readable explanation of the issue is set as the value. This is annoying to translate later. At least also include some error code string that is documented somewhere and isn't prone to change randomly.
- klodolph 3y agoI mean, you may end up just wanting something like, type UsernameError struct { name string reason string } func (e *UsernameError) Error() string { return fmt.Errorf("invalid username %q: %s", e.name, e.reason) } And reason can be "username cannot be empty" or "username may not contain '<'" or something like that. This is fine for lots of different cases, because it’s likely that your code wants to know how to handle “username is invalid”, but only humans care about why. I have personally never seen a Go codebase where you parse error strings. I know that people keep complaining about it so it must be happening out there—but every codebase I’ve worked with either has error constants (an exported var set to some errors.New() value) or some kind of custom error type you can check. Or if it doesn’t have those things, I had no interest in parsing the errors.
- Scarblac 3y agoI write mostly frontends. Sometimes the APIs I talk to give back beautiful English error messages - that I can't just show to the user, because they are using a different language most of the time. And I don't want to write logic that depends on that sentence, far too brittle.
- klodolph 3y agoRight—I think the “error code” here is going to be the error type, i.e., UsernameError, or some qualified version of that. It’s not perfect, but software evolves through many imperfect stages as it gets better, and this is one such imperfect stage that your software may evolve through. Including a human-readable version of the error is useful because the developers / operators will want to read through the logs for it. Sometimes that is where you stop, because not all errors from all backends will need to be localized.
- patrickkristl 3y agoem ai have a problem from cars
- the_gipsy 3y agoIt's not guaranteed at all, that's where go's zero-values come in. E.g. nested structs, un/marshaljson magic methods etc. How do you deal with that?
- stouset 3y agoEvery struct requiring its zero value to be meaningful is probably one of the worst design flaws in the language.
- randomdata 3y agoThere is no such requirement. Common wisdom suggests that you should ensure zero values are useful, but that isn't about every random struct field – only the values you actually give others. Initialize your struct fields and you won't have to consider their zero state. They will never be zero. It's funny seeing this beside the DRY thread. Seems programmers taking things a bit too literally is a common theme.
- stouset 3y ago> Initialize your struct fields and you won't have to consider their zero state. “Just do the right thing everywhere and you don’t have to worry!” You can’t stop consumers of your libraries from creating zero-valued instances.
- randomdata 3y agoThen the zero value is their problem, not yours. You have no reason to be worried about that any more than you are worried about them not getting enough sleep, or eating unhealthy food. What are you doing to stop them from doing that? Nothing, of course. Not your problem. Coq exists if you really feel you need a complete type system. But there is probably good reason why almost nobody uses it.
- stouset 3y ago
- swah 3y agoMy Go is rusty, do you mean not exporting the type "Username" (ie username) to avoid default constructor usage?
- mtlynch 3y agoIn Go, capitalized identifiers are exported, whereas lowercase identifiers are not. In the example I gave above, clients outside of the package can instantiate Username, but they can't access its "value" member, so the only way they could get a populated Username instance is by calling NewUsername.
- whimsicalism 3y agothe fact that this is some special “technique” really shows how far behind Go’s type system & community around typing is
- edflsafoiewq 3y agoYou can use new types with validation too. In fact the approaches seem to be duals. Parse, don't validate: string ParsedString untrusted source -------> parse --------------> rest of system Validate, don't parse: UnvalidatedString string untrusted source ------------------> validate -------> rest of system
- mtlynch 3y agoThe problem is that pattern "fails open." If anyone on the team forgets to define an untrusted string as UnvalidatedString, the data skips validation. If you default to treating primitive types as untrusted, it's hard for someone to accidentally convert an untrusted type to a trusted type without using the correct parse method.
- edflsafoiewq 3y agoThe dual problem would be any function which forgets to accept a ParsedString instead of a string can skip parsing. Both cases appear to depend on there being a "checkpoint" all data must go through to cross over to the rest of the system, either at parsing or at UnvalidatedString construction.
- mtlynch 3y ago>The dual problem would be any function which forgets to accept a ParsedString instead of a string can skip parsing. >Both cases appear to depend on there being a "checkpoint" all data must go through to cross over to the rest of the system, either at parsing or at UnvalidatedString construction. The difference is that if string is the trusted type, then it's easy to miss a spot and use the trusted string type for an untrusted value. The mistake will be subtle because the rest of your app uses a string type as well. The converse is not true. If string is an untrusted type and ParsedString is a trusted type, if you miss a spot and forget to convert an untrusted string into a ParsedString, that function can't interact with any other part of your codebase that expects a ParsedString. The error would be much more visible and the damage more contained. I think UnvalidatedString -> string also kind of misses the point of the type system in general. To parse a string into some other type, you're asserting something about the value it stores. It's not just a string with a blessing that says it's okay. It's a subset of the string type that can contain a more limited set of values than the built-in string type. For example, parsing a string into a Username, I'm asserting things about the string (e.g., it's <10 characters long, it contains only a-z0-9). If I just use the string type, that's not an accurate representation of what's legal for a Username because the string type implies any legal string is a valid value.
- zelphirkalt 3y agoI always understood "parse don't validate" a bit differently. If you are doing the validation inside of a constructor, you are still doing validation instead of parsing. It is safer to do the validation in one place you know the execution will go through, of course, but not the idea I understand "parse don't validate" to mean. I understand it to mean: "write an actual parser, whatever passes the parser can be used in the rest of the program", where a parser is a set of grammar rules for example, or PEG.
- mtlynch 3y agoI'm not a Haskell developer, so it's possible that I misunderstood the original "Parse, Don't Validate" post. >If you are doing the validation inside of a constructor, you are still doing validation instead of parsing. Why that would be considered validation rather than parsing? From the original post: >Consider: what is a parser? Really, a parser is just a function that consumes less-structured input and produces more-structured output. That's the key idea to me. A parser enforces checks on an input and produces an output. And if you define an output type that's distinct from the input type, you allow the type system "preserve" the fact that the data passed a parser at some point in its life. But again, I don't know Haskell, so I'm interested to know if I'm misunderstanding Lexi Lambda's post.
- kevincox 3y agoParse don't validate means that if you want a function that converts an IP address string to a struct IpAddress{ address: string } you don't validate that the input string is a valid IP address then return a struct with that string inside. Instead you parse that IP into raw integers, then join those back into an IP string. The idea is that your parsed representation and serializer are likely produce a much smaller and more predictable set of values than may pass the validator. As an example there was a network control plane outage in GCP because the Java frontend validated an IP address then stored it (as a string) in the database. The C++ network control plane then crashed because the IP address actually contained non-ASCII "digits" that Java with its Unicode support accepted. If instead the address was parsed into 4 or 8 integers and was reserialized before being written to the DB this outage wouldn't have happened. The parsing was still probably more lax than it should have been, but at least the value written to the DB was valid. In this case it was funny Unicode, but it could be as simple as 1.2.3.04 vs 1.2.3.4. By parsing then re-serializing you are going to produce the more canonical and expected form.
- tdeck 3y agoBut surely this is just another way of doing validation and not fundamentally "parsing"? If at the end you've just stored the input exactly as you got it, the only parsing you're potentially doing is in the validation step and then it gets thrown away.
- ezrast 3y agoImplementation-wise, yes, but the interface you're exposing is indistinguishable from that of a parser. For all your consumers know, you could be storing the username as a sequence of a 254-valued enum (one for each byte, except the angle brackets) and reconstructing the string on each "get" call. For more complex data you would certainly be storing it piecewise; the only reasons this example gets a pass are 1) because it is so low in surface area that a human can reasonably validate the implementation as bug-free without further aid from the type checker, and 2) because Go's type system is so inexpressive that you can't encode complex requirements with it anyway.
- draven 3y agoThe validation is not completely thrown away, since the type indicates that the data has been validated. I understand "parsing" as applying more structure to a piece of data. Going from a String to an IP or a Username fits the definition. I push my team to use this pattern in our (mostly Scala) codebase. We have too many instances of useless validations, because the fact that a piece of data has been "parsed"/validated is not reflected in its type using simple validation. For example using String, a function might validate the String as a Username. Lower in the call stack, a function ends up taking this String as an arg. It has no way of knowing if it has been validated or not and has to re-validate it. If the first validation gets a Username as a result, other functions down the call stack can take a Username as an argument and know for sure it's been validated / "parsed".
- kvnhn 3y agoThis is a variation on one of my favorite software design principles: Make illegal states unrepresentable. I first learned about it through Scott Wlaschin[1]. [1]: https://fsharpforfunandprofit.com/posts/designing-with-types-making-illegal-states-unrepresentable/ https://fsharpforfunandprofit.com/posts/designing-with-types...
- dang 3y agoRelated: Parse, don't validate (2019) - https://news.ycombinator.com/item?id=35053118 https://news.ycombinator.com/item?id=35053118 - March 2023 (219 comments) Parse, Don't Validate (2019) - https://news.ycombinator.com/item?id=27639890 https://news.ycombinator.com/item?id=27639890 - June 2021 (270 comments) Parse, Don’t Validate - https://news.ycombinator.com/item?id=21476261 https://news.ycombinator.com/item?id=21476261 - Nov 2019 (230 comments) Parse, Don't Validate - https://news.ycombinator.com/item?id=21471753 https://news.ycombinator.com/item?id=21471753 - Nov 2019 (4 comments)
- WuxiFingerHold 3y agoSo far I like the commonly used approach in the Typescript community best: 1. Create your Schema using https://zod.dev https://zod.dev or https://github.com/sinclairzx81/typebox https://github.com/sinclairzx81/typebox or one of the other many libs. 2. Generate your types from the schema. It's very simple to create partial or composite types, e.g. UpdateModel, InsertModels, Arrays of them, etc. 3. Most modern Frameworks have first class support for validation, like Fastify (with typebox). Just reuse your schema definition. That is very easy, obvious and effective.
- solumunus 3y agoThis doesn’t solve the problem. If I have a user type, inferred from a Zod schema: > { username: string; email: string } And a function which takes that type: > storeUser(user: User) There is absolutely nothing that guarantees that the user object has been parsed by Zod. You can simply: > storeUser({ username: “”, email: “no” }) And Typescript will not shout at you. The only way to comparably solve it with Typescript is to inject a symbol into the object during parsing which confirms it has been passed through the correct parser function. Personally, I just do basic type parsing on input data (usually request data) and more strict parsing where constraints like “is this a valid username, is this a valid email” during output (usually sending to the database). What happens in between I/O doesn’t matter much in many projects (CRUD), and in the places it does you can enforce more rigidity.
- angra_mainyu 3y agoWell, I use ajv and they have ways of applying format validation, so not just saying: "this is a string", but rather, "this is a string and must be a valid domain name". Now, if your complaint is rather that you can call whatever method and pass in your bogus data, I don't see the point in arguing that. It's your code, the only person who can stop you is you.
- solumunus 3y ago> Now, if your complaint is rather that you can call whatever method and pass in your bogus data This entire comment thread is a discussion about how to prevent that from being a possibility. The person I responded to threw their hat in with a Typescript solution that doesn’t achieve the goal being discussed. I was simply pointing this out.
- otabdeveloper4 3y ago> you have to remember to call the validator on 100% of code paths But copy-pasting the same lines of code in literally every function is the Golang Way. It makes code "simpler".
- HeavyStorm 3y agoEncapsulation saves lives.
- deleted 3y ago[deleted]
- hknmtt 3y ago[dead]
- qudat 3y agoLove it, I called it entity factories https://bower.sh/entity-factories https://bower.sh/entity-factories