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

api's json objects contain invalid keywords #5

Open
tomjack opened this issue Oct 19, 2012 · 7 comments
Open

api's json objects contain invalid keywords #5

tomjack opened this issue Oct 19, 2012 · 7 comments
Assignees

Comments

@tomjack
Copy link

tomjack commented Oct 19, 2012

E.g.:

user> (f/facets :restaurants-us {:select "cuisine"
                                 :filters {"locality" "New York"
                                           "region" "NY"}
                                 :limit 50})
{:cuisine
 {;; ...
  :pizza, sandwiches, subs 29,
  ;; ...
  }}

Note that the keywords contain spaces and commas. This looks weird, isn't readable (by the reader, I mean), and is somewhat difficult to work with. I think Strings make more sense here.

It seems clj-http can be made to return strings using :as :json-string-keys, but that causes an error here. Not sure why. Maybe oauth-clj is getting in the way? Of course the tradeoff would be that you get strings even in places where keywords would be appropriate. Hmm...

@tomjack
Copy link
Author

tomjack commented Oct 23, 2012

I realized it's unlikely oauth-clj is getting in the way — the problem is that keywords are expected in some places in the rest of the code.

Selective keywordization seems doable. We could implement a custom coercion method for clj-http. Of course this would break things for anyone who depended on keywords.

@ztellman
Copy link

Hey, sorry for the slow response on this. I think that we should strive for consistency w.r.t. strings vs keywords, and since keywords obviously don't work everywhere, we'll have to settle on strings. I'll be cutting a new version with the requisite changes in the next few days, let me know if you have any questions/comments.

@tomjack
Copy link
Author

tomjack commented Oct 31, 2012

Thanks, your response was quicker than expected.

I can understand the desire for consistency. Just in case, though, I want to note that by "selective keywordization" I did not mean "keywordize when the keywords would be valid, with a runtime check" but "keywordize when the json object's keys are semantically keywords (in which case, hopefully, they are always valid keywords)".

I'd rather have keywords where they make sense and strings elsewhere than strings everywhere. Of course this would require some thought (or inspection of results) by the user, but I don't see it causing too much trouble. I suspect the places where invalid keywords might be returned are exactly the places where a user probably expects strings.

@ztellman
Copy link

Could you give me an example of where keywords would be appropriate, in your opinion? Just going on the principle of least surprise, I agree not using keywords kind of sucks, but unless there's a simple heuristic for where they're used and where they're not, the mixed approach could easily be worse.

@tomjack
Copy link
Author

tomjack commented Oct 31, 2012

some places I expect keywords:

  • all keys for items returned from fetch/resolve
  • top-level keys from facets
  • top-level keys in multi results
  • top-level keys in geopulse
  • reverse-geocode

In general, I expect keywords when the key represents a schema field name or serves as structure for a response. Anytime the key is a piece of place data (whether or not all the values in that column happen to be valid keywords), I expect strings.

The only places I've noticed (so far..) where I expect strings are:

  • facet maps
  • geopulse commercial_profile (could easily expect keywords here, but since the docs show that category names are returned, I expect strings)

A heuristic that occurs to me is: if you could, theoretically, replace every instance of the key in all of Factual's data and everyone's code with a gensym, without losing anything except readability by devs, it's a keyword (assume people don't call name on these keywords...). Otherwise, it's a string. Keywords are, semantically, opaque labels, while strings are, well, strings.

I do understand though if you decide it's simpler to just use strings everywhere.

@ghost ghost assigned ztellman Mar 19, 2013
@dirtyvagabond
Copy link

@ztellman i think we let this fall through the cracks. mind picking it up after your Clojure West adventures?

@dcj
Copy link

dcj commented Mar 22, 2013

Just returning to this after a long time of non-use (busy with other things), previously I had used the "funny places" version....

I would like to strongly second the comments from @tomjack
As Tom stated:
"In general, I expect keywords when the key represents a schema field name or serves as structure for a response"
I definitely agree with this.

Actually, I do not yet understand why any key that you return would need to be a string. Everything returned by:

(map #(get % "name") (fact/schema :restaurants-us))

could easily be a keyword, and should be!

I have zero/no experience with the facet feature, but after a very cursory look at this, my POV is that it is kinda unimpressive if the way multiple items are specified is by stringifying them with comma separators. Really?
How about specifying a facet like this:

:select [:locality :region]

Same thing on the output side, I think. I confess I do not yet understand the query and the expected response of the example given above:

  :pizza, sandwiches, subs 29,

But if the intent is to state that there are 29 restaurants in the locality that serve "pizza, sandwiches, and subs",
where each of those is a recognized Factual cuisine/category, then I would propose the map returned look like:

{:cuisine
  {;; ...
   [:pizza :sandwiches :subs] 29,
   ;; ...
  }
}

If a user wants a prettier string/label/name than the keyword, then presumably there are ways to go from the keyword (i.e the keywordized "name") to the nice string which is/should-be the value of "label" in your schema.

In summary, I have a stronger opinion than Tom, offhand I cannot think of any place where string keys would be appropriate in a request, and the only place I can imagine they might be appropriate in a response is when the keys in the response are from some "real word" data which is not part of your schema.....

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

4 participants