3 ms·
If a field is an int and the incoming data has a string the parsing shouldn't succeed - instead it should give an error like "expected an int but saw a string"
by jayshua 5y ago
If a field is an int and the incoming data has a string the parsing shouldn't succeed - instead it should give an error like "expected an int but saw a string". Maybe include the string it saw in the error. Add in the JSON path for bonus debugging points.
You shouldn't get to the point of having a CreateTaskRequest with a 0 that shouldn't be there.
- bcrosby95 5y agoI understand and have done that before with XyzRequest classes. What I'm saying is the field shouldn't be an int. It's not the proper level of abstraction for what it represents, and any of: tossing out, transforming, or hiding under layers of abstraction the data that comes into your system like this is a bad idea and is the sort of thing that makes people hate OOP. Especially when perfectly fine alternatives exist for this.
- jayshua 5y agoInteresting. I'd say the proper level of abstraction for the incoming JSON is some structure that can represent all valid JSON. Then you convert from that JSON thing into the form you expect - in this case a CreateTaskRequest (with int fields and whatnot) - which would be the proper level of abstraction for the code that deals with creating tasks. Is that substantially different from what you suggest?
- jwhitlark 5y agoMy concern with all-or-nothing validation of the sort you're suggesting is that sometimes the invalid data is data that isn't your concern and doesn't matter to the task you're trying to accomplish. It doesn't even have to be a mistake on the sender's side; maybe something evolved, maybe you made the mistake. I prefer defining the minimum I need for a specific function, instead of defining exactly what I expect. For much the same reasons as we scribble in the margins sometimes.
- lolinder 5y agoWhen encoding types for requests like this, I'm exactly as strict as I need to be: no more, no less. Any assumption about the shape of the data that my code will make is encoded in the parser, but the parser is very generous with everything else. For example, if a request has a field I didn't expect, that's not an error, I simply ignore that field because I obviously didn't need it. Likewise if requests start using a more specific schema, such as now sending only positive integers. If, however, the schema changes such that a field I rely on is dropped or changes type, my code will eventually fail anyway as soon as it makes an assumption about the data. Why not fail at the point of entry instead of saving it for later?
- jwhitlark 5y agoThat sounds like it might work. I was commenting based on my experience of people deciding there had to be a single representation for an idea/type no matter what it was used for, across multiple projects and repositories. I was not dismissing appropriate validation at a boundary, just trying to pointing out that, at least in my experience, people get way too wound up about enforcing unnecessary consistency.