dlang-community / std_data_json

Phobos candidate JSON implementation.
25 stars 13 forks source link

About naming #27

Open bubnenkoff opened 8 years ago

bubnenkoff commented 8 years ago

For me naming looks very strange. For example toJSON Converts the given JSON document(s) to its string representation, but by it's name I expect that it should convert string to json.

wilzbach commented 8 years ago

toJSON is a rather common name. That's the name in JSON's origin language JavaScript uses it too, but I agree it's rather confusing.

Imho ideally it would just extend std.conv, s.t. we can use `myjson.to!string and mystr.to!json.

s-ludwig commented 8 years ago

The problem is that since JSONValue forwards all methods/operations to the contained value, this would create an ambiguity (i.e. do you want the string value of the contained value, or the JSON string representation of it?). toJSONString would be unambiguous, but also quite long.

The other alternative would be to not forward operations to the contained value, which makes working with JSONValue quite a bit more convoluted.

zoujiaqing commented 8 years ago

change toJSON() to toString()

zoujiaqing commented 8 years ago

change JSONValue to JSONObject ?

s-ludwig commented 8 years ago

The problem is that many of these functions are highly ambiguous. Does toString() convert the represented value to string, or the whole thing to a JSON string? toJSONString would be the clearest choice, the trade-off is its length. But JSONObject would be the wrong term, because a JSONValue can also contain numbers, strings, arrays and Booleans in addition to objects.

bubnenkoff commented 8 years ago

@s-ludwig length is not so much problem, it's really better to add several letters, than get problem when people do not understand what function do by it's name.

What do you understand by whole thing to a JSON string ?

zoujiaqing commented 8 years ago

can use toJsonString() and toJsonValue() names.

s-ludwig commented 8 years ago

The problem with "Json" is that this needs to adhere to http://dlang.org/dstyle.html, which specifies that acronyms must be written all-uppercase. But I agree that it reads a lot mode lightweight. For that reason I've used another rule for vibe.d, where acronyms that are pronounced as a single word are still written in camel case, but that can't be applied here.

length is not so much problem, it's really better to add several letters, than get problem when people do not understand what function do by it's name.

I agree, but there have been strong opposing voices in the review process. It was argued to drop "JSON" from all names, which would result in code such as Value val = parse(str); that is completely opaque. IMO it doesn't make sense to try to safe a few letters for code that is relatively rare, at the expense of requiring the reader to scan for imports to even get what it does.

What do you understand by whole thing to a JSON string ?

toJSONValue("\"foo\"").toString() could yield either "foo" or "\"foo\"". Since JSONValue is a transparent algebraic data type at its core, it's not all that clear what should happen. The same goes for to!T integration. The problem is that the kind of algebraic data type that forwards operations to the stored type can be very convenient, but it comes at the cost of introducing some ambiguity/the need for more explicit names.

bubnenkoff commented 8 years ago

@s-ludwig I can't remember situations where additional quotes like "\"foo\"" was helpful. In most cases they only add problems and different kind of errors.

Maybe it's better to make generation of "foo" by default?

Sometimes it's very hard to understand where I should to use std.conv and where tail toString method

s-ludwig commented 8 years ago

Well, "\"foo\"" would be valid JSON syntax, whereas "foo" would just be the string contents itself. The former is what toJSON does. toJSONString would make that really clear, but for toString you can't really tell up front which of the two will be returned.