json-ld / json-ld.org

JSON for Linked Data's documentation and playground site
https://json-ld.org/
Other
859 stars 152 forks source link

7.6 section in json-ld 1.0 document is not clear (or: clarify which ifs are actually "if else") #561

Closed cwebber closed 6 years ago

cwebber commented 6 years ago

Yeah yeah I should look at 1.1 to see if it's clarified but I'm most of the way across the ocean with this 1.0 implementation and I'm not ready to try to pivot / audit against 1.1. So this is against 1.0's API spec!

Here's the thing, the compaction algorithm seems pretty clear and I was able to do it without looking at a reference implementation all the way up to 7.6, where (and it could be my brain is melting due to a marathon hack session) but it seems to me that the text is very much not clear. In all the other sections of this you were building up a "result" dictionary and the end of the section says "oh now append this thing to the result with this key and this value" and great, awesome, horray. But I'm confused by 7.6:

I'm guessing it's probably the former. Am I right?

gkellogg commented 6 years ago

@cwebber said:

Is this code like "if 7.6.4 / if 7.6.5 / else 7.6.6" or is it "if 7.6.4 / else if 7.6.5 / else 7.6.6"?

It's the former, as it is written. Either way, we're setting up compacted item. In 7.6.4, if it's a list object and container is @list, it will keep an array form and fall through to 7.6.6 where it's added to result. If it's not a list object, then it's turned back into a list representation via {"@list": _compacted item_}, it may then be processed as container @language or @index, or fall into 7.6.6 again, but at least now it's back into a list form.

If you find a use case where the algorithm doesn't work, that would be good to know.

The 1.1 algorithm has more pieces, because we're also looking for graph objects as well as list objects, and container types can include @id and @type and can also include @set.

cwebber commented 6 years ago

Ok that's great! Thanks for the clarity... that's enough to unblock me :)

I think the reason this was hard for me is that there is no consistent "if else" usage in the document. If there was, this bit would be clear. For example, even see 1, 2, 3 in the compaction algorithm (even though that one's easy to infer), or likewise 7.2.2.1.1, 7.2.2.1.2, 7.2.2.1.3 (a bit more work)... it's not clear, based on the text alone, if the second "if" is a fresh "if" or an "if else". You can often figure these things out based on the context, but that requires extra thinking and parsing than sometimes is desirable or at least necessary when knee deep ;)

So I think at this point, the remaining point of this issue would be: could we have a consistent "if else" phrasing, and could we make use of it?

gkellogg commented 6 years ago

Basically, the spec uses If, If, Otherwise, like the following:

if foo {}
if bar {}
else {}

The other case would be If, Otherwise If, Otherwise:

if foo {}
else if bar {}
else {}

This seems like a fairly straightforward interpretation for someone who's doing an implementation. However, if you can think of a better way to state it that would be less confusing, please make a suggestion. If you find an inconsistency, we should certainly fix it.

cwebber commented 6 years ago

Okay... you're right that "otherwise if" is used elsewhere. Thanks for your help, closing this!