Change parseJson to perform type validation #168
Conversation
| [$.String, $Maybe($.Any)], | ||
| encase(JSON.parse)); | ||
| [TypeRep, $.String, $Maybe(a)], | ||
| function(type, json) { return filter(is(type), encase(JSON.parse, json)); }); |
There was a problem hiding this comment.
Let's name the second parameter s rather than json, as it's not necessarily a valid JSON string.
|
This is very nice, Aldwin! |
| //. otherwise. | ||
| //. Takes a [type representative](#type-representatives) and a string which | ||
| //. may or may not be valid JSON, and returns Just the result of applying | ||
| //. `JSON.parse` to the string *if* the result is of the specified type |
There was a problem hiding this comment.
What do you think about the cursive if? I thought it's a nice way to break up the long sentence, but I'm not sure about it.
There was a problem hiding this comment.
It works for me, but the one thing that's not clear from this description is the handling of invalid strings.
What do you think of this phrasing?
… and returns Just x if the given string is a JSON representation of x, a value of the specified type (according to is); Nothing otherwise.
There was a problem hiding this comment.
I considered it, but I think only specifying the latter is good enough, because it implies the first. That combined with the code example makes it pretty clear. I'm worried going into the specifics take away from the ease of reading and clarity.
There was a problem hiding this comment.
Okay. I'm happy with your current wording also.
|
@davidchambers Did you see I did the thing? |
|
Thanks, @Avaq. Now that we've merged sanctuary-js/sanctuary-def#38 I can publish a minor release of sanctuary-def and a patch release of this project (to fix #165). That will free us to start merging changes for the v0.10.0 release. |
|
I see, so we're awaiting the release of |
|
Please rebase this branch when you get a chance. |
Avaq
force-pushed
the
av-validated-parsejson
branch
from
d61b9c3 to
30b3b7a
Compare
|
ℹ️ @davidchambers Noticed the rebase? I'm not sure if it would've notified you. |
| '1) "String" :: String\n' + | ||
| '\n' + | ||
| 'The value at position 1 is not a member of ‘TypeRep’.\n')); | ||
| throws(function() { S.parseJson(Array, [1, 2, 3]); }, |
There was a problem hiding this comment.
Mind adding an empty line between these blocks?
Adds a second argument to the `parseJson` function: A TypeRep which the result from parsing the JSON is validated against. This closes #150.
Avaq
force-pushed
the
av-validated-parsejson
branch
from
30b3b7a to
97f2327
Compare
|
🌳 Thanks very much, @Avaq! |
Adds a second argument to the
parseJsonfunction: A TypeRep which the result from parsing the JSON is validated against. This closes #150.