rdfjs / data-model-spec

RDF/JS: Data model specification – A specification of a low level interface definition representing RDF data independent of a serialized format in a JavaScript environment.
http://rdf.js.org/data-model-spec/
73 stars 12 forks source link

Term normalization for `.quad` function? #145

Open blake-regalia opened 5 years ago

blake-regalia commented 5 years ago

While this issue is related to the .equals function, it actually exists outside the scope of the other discussions we've had. This one might be addressed by amending the definition of the .quad function itself, but right now there is asymmetry in the naive implementation:

let realTerm = factory.namedNode('http://ex.org/red');
let fakeTerm = {
    termType: 'NamedNode',
    value: 'http://ex.org/red',
};

let realQuad = factory.quad(realTerm, realTerm, realTerm, realTerm);
let fakeQuad = factory.quad(fakeTerm, fakeTerm, fakeTerm, fakeTerm);

realQuad.equals(fakeQuad);  // `true`
fakeQuad.equals(realQuad);  // throws exception

Most importantly, without specifying some needs for normalizing arguments passed to .quad (as it is now), we will see different behavior, i.e., inconsistencies in the results of .equals, between implementations that normalize terms passed to .quad and those that don't.

I understand that the parameter type is specified as Term to the .quad function, but I think it's important to at least consider and discuss these circumstances. Possibly related to #104 and #137

RubenVerborgh commented 5 years ago

We shouldn't overthink. .quad expects Term instances. If passed something else, behavior is undefined. Normalization through #137.

I don't want to discourage good discussions, do keep coming up with corner cases. However, many issues are full of "fake Term" corner cases, whereas I yet have to see the first such fake object in the wild. There is no good reason to create them—not even in transit, because we have the RDF syntaxes for that.

Suggestion: let's assume that fake Term instances won't be a thing. I think it's prudent to guard for undefined or null (which I have seen in the wild), but I happily give up on these if that means that other non-instances are no longer considered either, for simplicity's sake.