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

JSON package fails to reconstruct a new record #451

Open
IanTrudel opened this issue May 1, 2024 · 6 comments
Open

JSON package fails to reconstruct a new record #451

IanTrudel opened this issue May 1, 2024 · 6 comments

Comments

@IanTrudel
Copy link

The JSON package fails to reconstruct a new record when the said record has never been used in the code. I suspect the compiler remove unused records and therefore cannot reconstruct the record. The current JSON implementation uses proc for both record and class. A potential solution would be to use constructor() instead.

A working example:

link ximage

import json

record a(b, c)

procedure main()
   ra := a(1, 2)
   ja := utoj(ra)
   jra := jtou(ja)
   jra.b := 3

   write(ja)
   write(ximage(jra))

   raa := jtou("{\"__unirecord__\":\"a\",\"b\":4,\"c\":5}")
   raa.c := 9
   write(ximage(raa))
end

{"__unirecord__":"a","b":1,"c":2}
R_a_3 := a()
   R_a_3.b := 3
   R_a_3.c := 2
R_a_5 := a()
   R_a_5.b := 4
   R_a_5.c := 9

Fails to create a new record n:

link ximage

import json

record a(b, c)
record n(m, o)

procedure main()
   ra := a(1, 2)
   ja := utoj(ra)
   jra := jtou(ja)
   jra.b := 3

   write(ja)
   write(ximage(jra))

   rn := jtou("{\"__unirecord__\":\"n\",\"m\":8,\"o\":7}")
   #rn.c := 9 # Would throw an error
   write(ximage(rn))
end
{"__unirecord__":"a","b":1,"c":2}
R_a_3 := a()
   R_a_3.b := 3
   R_a_3.c := 2
T4 := table(&null)
   T4["__unirecord__"] := "n"
   T4["m"] := 8
   T4["o"] := 7

It will not fail when at least one record has been created, anywhere in the code:

link ximage

import json

record a(b, c)
record n(m, o)

procedure main()
   ra := a(1, 2)
   ja := utoj(ra)
   jra := jtou(ja)
   jra.b := 3

   write(ja)
   write(ximage(jra))

   rn := jtou("{\"__unirecord__\":\"n\",\"m\":8,\"o\":7}")
   rn.o := 9
   write(ximage(rn))

   # Using the record anywhere works
   tn := n(0, 0)
end
{"__unirecord__":"a","b":1,"c":2}
R_a_3 := a()
   R_a_3.b := 3
   R_a_3.c := 2
R_n_2 := n()
   R_n_2.m := 8
   R_n_2.o := 9

A potential solution:

link ximage

import json

record a(b, c)
record n(m, o)

procedure main()
   ra := a(1, 2)
   ja := utoj(ra)
   jra := jtou(ja)
   jra.b := 3

   write(ja)
   write(ximage(jra))

   rn := constructor("n", "m", "o")(8, 7)
   rn.o := 9
   write(ximage(rn))
end
{"__unirecord__":"a","b":1,"c":2}
R_a_3 := a()
   R_a_3.b := 3
   R_a_3.c := 2
R_n_1 := n()
   R_n_1.m := 8
   R_n_1.o := 9
@StephenWampler
Copy link
Collaborator

Interesting problem - good catch!

The use of constructor() is really just hiding the creation of the record instance, however. So, invoking utoj() on the result still won't recreate the record unless a record instance is explicitly created somewhere in the code as in your previous example. Or am i missing something? Try your last example with the following lines added to the end of main():

   jn := utoj(rn)
   write("jn: ",jn)
   jrn := jtou(jn)
   write("jrn: ",ximage(jrn))
   jrn.m := 15

``

@IanTrudel
Copy link
Author

Interesting problem - good catch!

The use of constructor() is really just hiding the creation of the record instance, however. So, invoking utoj() on the result still won't recreate the record unless a record instance is explicitly created somewhere in the code as in your previous example. Or am i missing something?

Exactly. The default behaviour should perhaps be the creation of a record instance, when it's not found, due to the nature of records — they are just a structure with fields.

Try your last example with the following lines added to the end of main():

   jn := utoj(rn)
   write("jn: ",jn)
   jrn := jtou(jn)
   write("jrn: ",ximage(jrn))
   jrn.m := 15

Mind blown. To be fair, the JSON package is unlikely to consider dynamically created records in its current implementation.

@StephenWampler
Copy link
Collaborator

To me, this is an example of a 'premature' optimization. Information about a record shouldn't be discarded just because there's no explicit creation of an instance of that record because of dynamic invocation (i.e. proc()). If it hadn't been discarded, JSON would [probably] have worked just fine because proc() would have worked.

The fact that proc() works or doesn't work because of something that is referenced later in the code is a flag that this optimization shouldn't have been done. Note that the statement: if 1 = 0 then n(12.13) is sufficient for proc() to work even though the record creation will never occur and could, conceivably, be optimized out.

Agree about dynamically created records - except that the JSON strings could be read in from a source created by a different program [which introduces other issues, mind you, such as both programs must have the same record declarations!].

@Jafaral
Copy link
Member

Jafaral commented May 1, 2024

FYI, those two json library functions were assume you want to encode/decode Unicon data structures. I started using the library recently and realized I didn't care about encoding the structures, instead I care about encoding the data. I added a new function tojson(). So So instead of doing utoj() -> jtou(), I do ``tojson() -> jtou()`

Try:

   jn := tojson(rn)
   write("jn: ",jn)
   jrn := jtou(jn)
   write("jrn: ",ximage(jrn))

@StephenWampler
Copy link
Collaborator

StephenWampler commented May 1, 2024

Of course, tojson() produces something simple that jtou() converts into a table [as I'm sure you know, Jafari], so the two operations are not inverses. Substituting tojson() for utoj() in the 'bad' example:

-> t1c
{"b":1,"c":2}
T3 := table(&null)
   T3["b"] := 1
   T3["c"] := 2

Run-time error 107
File t1c.icn; Line 14
record expected
offending value: table_3(2)
Traceback:
   main()
   {table_3(2) . b} from line 14 in t1c.icn

Interestingly, in the original 'bad' example from Ian, replacing the record declarations with class declarations shows the same 'missing' constructor if no instance of a class is referenced in the code. That's not really a surprise, since classes are records internally. What caught me surprise is the tojson() -> jtou() produces different problems depending on whether record or class declarations are used. For the class declaration version:

-> t1c
{"b":1"c":2}
[json] Unable to open file "{\"b\":1\"c\":2}"
&null

Run-time error 107
File t1c.icn; Line 14
record expected
offending value: &null
Traceback:
   main()
   {&null . b} from line 14 in t1c.icn

@Jafaral
Copy link
Member

Jafaral commented May 2, 2024

Of course, tojson() produces something simple that jtou() converts into a table [as I'm sure you know, Jafari], so the two operations are not inverses.

Yup! the intention is to encode data, not structures as I mentioned. In most practical use cases the other side isn't even a unicon program. A JSON object is dictionary (value field pairs), if you want to store that in a record in your application, then it is up to the application developer.

I also understand the utility of the other semantics to encode data structures if used between two unicon applications and only between two unicon applications. That json encoding semantics is wrong outside that use case.

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

3 participants