Skip to content

Add more details contrasting fromString and jsonParser #45

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.

Sign up for GitHub

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

Merged
thomashoneyman merged 2 commits into from
Nov 6, 2020
Merged

Add more details contrasting fromString and jsonParser #45

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c4b237c
Add more details contrasting `fromString` and `jsonParser`
milesfrain Oct 19, 2020
78de086
Clarify json object language
milesfrain Nov 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/Data/Argonaut/Core.purs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,12 @@ foreign import fromBoolean :: Boolean -> Json
-- | Construct `Json` from a `Number` value
foreign import fromNumber :: Number -> Json

-- | Construct `Json` from a `String` value. If you would like to parse a string
-- | of JSON into valid `Json`, see `jsonParser`.
-- | Construct `Json` from a `String` value.
Copy link
Contributor

@thomashoneyman thomashoneyman Oct 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this be clearer if it said "Construct a Json string from a String value" to make it clear it's just producing a string, and not bonafide JSON with strings, numbers, objects, etc.?

Copy link
Member Author

@milesfrain milesfrain Oct 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Json string" makes me think it produces the string representation of the json contents - like what you'd get from stringify.
What about changing to: "Construct the Json representation of a String value"?
I feel like the output is still bona fide JSON, even if it's very simple JSON in this case.
Also, if we change that first line, we should probably change all the other fromX docs.

Copy link
Contributor

@thomashoneyman thomashoneyman Nov 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with that phrasing. And you're right, it's still bona fide JSON.

-- | Note that this function only produces a `Json` object containing a single piece of `String`
Copy link
Contributor

@thomashoneyman thomashoneyman Oct 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this just produces a string, not an object -- unless 'JSON object' can mean any JSON, not just objects.

Copy link
Member Author

@milesfrain milesfrain Oct 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think from JavaScript's perspective, a string is also technically an object. Really, it seems that everything is an object, which is why we can apply these instance methods:

"hello".charAt(3) // "l"

z = 1.234567
z.toPrecision(3) // "1.23"
typeof z // "number"

However, the docs state that String, Number, etc. are not Object but primitives which by definition have no methods.

a primitive (primitive value, primitive data type) is data that is not an object and has no methods. There are 6 primitive data types: string, number, bigint, boolean, undefined, and symbol.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures
https://developer.mozilla.org/en-US/docs/Glossary/Primitive

So I don't know what explains this inconsistency with the definitions of objects vs primitives and observed behavior. Are the docs incorrect? Is there some magic coercion/casting happening?

Back to JSON, the acronym also suggests that you always get an object (perhaps in the "everything is an object" sense), even if it's not key-value Object data (which has nothing to do with JS's "has methods" definition of Object - I'd rather see this named as dict or map when referring to json data). But I see how writing "object" perpetuates confusion, so I'd be up for rewriting as something like the following:

Note that this function only produces Json containing

Not sure about the exact phrasing, since it sounds pretty weird if you substitute with another type, e.g. "produces Number containing".

In an alternate universe, we'd be working with JSAN "JavaScript Array Notation", which I'd argue is no worse than the current name.

Copy link
Contributor

@thomashoneyman thomashoneyman Nov 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, I'm thinking here about object in the data sense (map, dict, etc.), not object in the object-oriented programming sense. I think of JSON as being comprised of strings, numbers, booleans, arrays, objects, and null, which I believe is a common way to think of it, in which case referring to an array as an 'object' would confuse me a little.

Note that this function only produces Json containing

I think this is fine phrasing.

-- | data (similar to `fromBoolean`, `fromNumber`, etc.). These "fromX" functions are often
-- | used to build-up JSON objects manually, as opposed to using `encodeJson`.
-- | This function does NOT convert the `String` encoding of a JSON object to `Json` - For that
Copy link
Contributor

@thomashoneyman thomashoneyman Oct 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
-- | This function does NOT convert the `String` encoding of a JSON object to `Json` - For that
-- | This function does NOT parse a string of JSON data into the equivalent `Json` -- for that

Copy link
Contributor

@thomashoneyman thomashoneyman Oct 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little wary of using the term 'object' here as it may lead a reader to believe this only applies to literal objects

Copy link
Contributor

@thomashoneyman thomashoneyman Nov 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to mention this, but I'd like to take out this line:

These "fromX" functions are often used to build-up JSON objects manually, as opposed to using encodeJson.

as there are more libraries for working with JSON than argonaut-codecs (even though it's the one bundled in to the 'argonaut ecosystem') and this library is more of a common base JSON type and utility functions for any other JSON libraries to work with.

-- | purpose, you'll need to use `jsonParser`.
foreign import fromString :: String -> Json

-- | Construct `Json` from an array of `Json` values
Expand Down