Hacker News Reader

Comment by bcrosby95 | Hacker News Reader

I like this design for making requests, but I don't like it for serving requests. Because if you don't just make every field a String in CreateTaskRequest, and you want to examine it in some way, then you're losing information. E.g. if you make a field an int, and they provide a non-int value, your CreateTaskRequest can't hold that value so its just gonna have a 0.

A class full of strings feels like a code smell - but that is the proper representation of serving the request. And the alternative is needlessly restrictive. Ultimately it feels like pointless ceremony.

View on HN

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.

I 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.

Interesting. 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?

My 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.

When 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?

That 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.

Reply on news.ycombinator.com