Add undefined_as_null to decoder

[Licenser]

Add undefined_as_null as a decoder option to resolve #26

Also adds undefined to json_value() for correctness as it could both be a valid input as well as output value now.

[pichi]

Your PR doesn't solve the issue #26.

1> X = [null, undefined], jsone:encode(X, [undefined_as_null]).
<<"[null,null]">>
2> jsone:decode(v(1), [undefined_as_null]).
[undefined, undefined]
3> v(2) =:= X.
false

I think it is not a good idea. You can always easily postprocess jsone:decode/1,2 result according your requirements which could be very specific. Where undefined_as_null option is pretty reasonable for jsone:encode/2 the same doesnt apply for jsone:decode/2. I doubt it is reasonable for general use. The null_as_undefined would be more appropriate name of the option and the #26 matching encoded and decoded data is definitely not the reason.

[Licenser]

No that's a case it does not cover, as it's impossible the information if null was undefined or a null is lost when it is added to JSON, but it is far more of an edge case than an application using undefined

undefined is closest Erlangs canonical representation of what null is in JSON, many applications do depend on that value to deal with unset or nonexistent values. The encoder already honors this I don't see how the decoder allowing for this is a bad idea.

null on the other hand is unheard of in Erlang land. Personally, I'd completely argue against using it as it seems odd to introduce a javascriptism like that into an Erlang program where it will probably cause more confusion than good, but that's just me.

Post processing of cause is possible, so would be pre-processing for the encoder. But it is neither trivial nor is it performant, which is what I suspect is the reason for undefined_as_null in the first place.

I honestly don't see any reason not to do it, but I'm curious to hear why you think it is bad?

Regarding undefined_as_null vs. null_as_undefined I thought of the naming more in the line of "Undefined means null in Erlang land" then as "Translate undefined to null" and kept the options matching. I think it's nicer if the same option can be supplied to both functions (less to learn for someone wanting to use it) but I'm happy to change it if it's a huge concern.

[sile]

undefined_as_null vs. null_as_undefined

In my sense, the latter is more natural. I usually associate A_as_B with "Converts A as B" (e.g., the warnings_as_errors option in the compiler module). So if there is no documentation, I will think the name of undefined_as_null means "Converts undefined values as null values".

Apart from it, basically I agree with @Licenser.

What is worrisome is that the undefined_as_null decoding option has a possibility of losing information of the input JSON. On the encoding side, we are able to control the input data perfectly. Conversely, on the decoding side, we can not predict which JSON data will be passed in advance. With undefined_as_null option, if a JSON that includes both null and undefined is passed, it will be impossible to know about that from the decoding data. Even if the undefined value was generated unintentionally (accidentally), the decoding side program will accept it without noticing the bug.

I think it causes no problem in almost all cases. But I'm glad to it if the risk of losing information is noted in the document (EDoc) about the option.

[Licenser]

Hi @sile I'm happy to update the docs on the option!

But I fear I have to ask for clarification as I fail to see where the information could get lost. undefined isn't a JSON term so it couldn't be passed in and it seems like jsone:encode doesn't seem to do partial decoding. I'm sure it's something obvious I'm missing, mind to give me a hand to catch up on hat? :)

[sile]

Oops, I confused JSON with JavaScript ... It will be appreciated if you ignore the latter part.

[Licenser]

What later part? I saw nothing ;)

I've put some better documentation for my changes in. If there is anything else you need to be fixed/improved let me know!

[sile]

I came to feel that this name is not so bad, but it seems better to factor out the options into common_option/0 as follows (for emphasizing that it is an option used in both in the encoder and the decoder):

-type common_option() :: undefined_as_null.
%% `undefined_as_null': <br />
%% - Treats `undefined' in Erlang as the conversion target for `null' in JSON. This means that `undefined' will be encoded to `null' and `null' will be decoded to `undefined'<br />

-type decode_option() :: ... | common_option().
-type encode_option() :: ... | common_option().

[Licenser]

Very good point, updated the PR

[sile]

Thanks. If there aren't any further comments within a few days, I will merge this PR.