Skip to content
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

Open
cooncesean opened this issue Dec 17, 2014 · 9 comments
Open

Dealing With "ChoiceFields" #57

cooncesean opened this issue Dec 17, 2014 · 9 comments

Comments

@cooncesean
Copy link

Overview and Model Layer

We have models using SmallInt fields to store references to their more verbose string counterparts. Lets take a look at our User model for example:

# django `models.py` file -- apologies for the framework specific example
class User(models.Model):
    """A representation of a user account."""
    GENDER_UNSPECIFIED = 0
    GENDER_MALE = 1
    GENDER_FEMALE = 2    
    GENDER_CHOICES = (
        (GENDER_UNSPECIFIED, 'unspecified'),
        (GENDER_MALE, 'male'),
        (GENDER_FEMALE, 'female')
    )
    gender = models.SmallIntegerField(choices=GENDER_CHOICES, default=GENDER_UNSPECIFIED)

# Create a new user and specify its gender
user = User.objects.create(gender=User.GENDER_MALE)
user.gender == 1

# Django has utility that implements `get_FOO_display` to return the 
# more verbose string counterpart of the currently set value
user.get_gender_display() == 'male'

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:

user.gender = 'male'
user.save()  # would raise a ValueError

user.gender = User.GENDER_FEMALE
user.save()
user.gender == 2

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:

  1. Is it reasonable to return/accept gender as an Integer in our requests -- or should we be using the more human readable string counterpart instead?
  2. What is the best way to convey all of the available gender options to a consumer?
  3. What is the best way to handle changes to the key/val mapping? eg: What if we create a fourth gender option called Squid?

Currently, a GET request to our endpoint looks like this:

GET /api/users/
HTTP 200 OK
Content-Type: application/json ;utf-8

[
    {
        "url": "http://project.com/api/users/1/", 
        "gender": 2
    }
]
@kwood
Copy link

kwood commented Dec 17, 2014

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!)

  1. We should use string literals, both for read and write, and they should be properly validated in the serializer
  2. Documentation, this should be possible to document automatically I would think.
  3. Ideally adding another item to the choice field in the model should be all you need to do. Serializer and documentation should pick that up automatically. If it's not possible out of the box, it should be trivial to write such a serializer

@tboyce12
Copy link

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.

@kwood
Copy link

kwood commented Dec 17, 2014

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

@Leland-Takamine
Copy link

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
I don't see how choice parameters differ from any other part of an API spec. In other words, I think we should document the spec in the same way we document all other requirements of the API. If you are looking a more robust way of enforcing type-safety (as opposed to relying on documentation), I think the only other option would be Protocol Buffers (https://code.google.com/p/protobuf/), which I don't think are used by anyone besides Google and Square.

@alanhogan
Copy link

Definitely should expose the display strings in the API, for the same reason you should always use the constants to assign to them

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.

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.

Easily solved by server-side validation (for client errors) or by tests — you wouldn’t accept -999 just like you wouldn't accept mail, and you can assert that the field is male

@alanhogan
Copy link

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”)

@geemus
Copy link
Member

geemus commented Dec 18, 2014

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.

@alanhogan
Copy link

Excellent, well said. Thanks for chiming in, @geemus. (Looks like an R. Stevens avatar, yeah? Nice.)

@geemus
Copy link
Member

geemus commented Dec 18, 2014

@alanhogan hah, good eye. R. Stevens did in fact make me this fine avatar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants