-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dealing With "ChoiceFields" #57
Comments
Definitely should expose the display strings in the API, for the same reason you should always use the constants to assign to them (never use integer literals!)
|
I like the idea of passing choice fields as an integer instead of a string because passing around integers is less prone to subtle errors such as capitalization, string encoding, spelling, etc. Just make sure to document the possible choices and corresponding integer values. |
https://github.com/tomchristie/django-rest-framework/blob/master/rest_framework/fields.py#L970 <-- Looks like the ChoiceField serializer should do this out of the box |
I think human readable values make more sense. @tboyce12 On the contrary I think integers are more prone to errors as integers by themselves have no meaning. re: Exposing choices |
Strongly agree — huge advantage of JSON as an API format is readability and I just can’t see any reason whatsoever to obfuscate meaning in API design.
Easily solved by server-side validation (for client errors) or by tests — you wouldn’t accept -999 just like you wouldn't accept |
Which code, used to retrieve a localized gender string based on the gender of the user as returned from the API, has a bug? switch user.gender
when 1 then 'gender.male'
when 2 then 'gender.female'
else 'gender.unknown' or switch user.gender
when 'female' then 'gender.male'
when 'male' then 'gender.female'
else 'gender.unknown' (The correct answer is “definitely the second code block, and we have no way of knowing with the first code block without a reference”) |
I almost always lean toward human-readable. I think any time (within reason) that you can do a small amount of extra work to transfer more context/info, you probably might as well. Additionally, I think this is a case where either the server can do a bit more work or EVERY client can do a bit more work. In those cases I usually feel that doing the work once/centrally is probably preferred. In this case I think there is value in strings instead of integers to abstract away from the underlying stuff (in case you added new options and/or needed to change the meaning of existing values for some reason). I'm not sure, off hand, of a good way to necessarily encode this info back to the user. In any documentation I would make it clear that there is an enum of valid options and clearly list them. Then in usage, I would raise an error (probably 422) if somebody posted one that didn't match. Ideally the body of that response could then clearly spell out something like "gender must be one of ...". Hope that helps, happy to discuss further if it would be helpful though. |
Excellent, well said. Thanks for chiming in, @geemus. (Looks like an R. Stevens avatar, yeah? Nice.) |
@alanhogan hah, good eye. R. Stevens did in fact make me this fine avatar. |
Overview and Model Layer
We have models using
SmallInt
fields to store references to their more verbose string counterparts. Lets take a look at ourUser
model for example:Behind the scenes the data for the
gender
field is stored as an integer (for a multitude of reasons). When we render this field, we tend to expose the more "human readable" string counterpart, but when we manipulate the data, we do so by using its "key", like so:API Design Questions
We're trying to figure out the best way to expose "choice" fields like this over our API and had a few questions:
gender
as anInteger
in our requests -- or should we be using the more human readable string counterpart instead?gender
options to a consumer?Squid
?Currently, a
GET
request to our endpoint looks like this:The text was updated successfully, but these errors were encountered: