Quad interface: when does equals return true?

[k00ni]

The spec says:

equals() returns true if and only if the argument is a) of the same type b) has all components equal.

The only parameter is of type Quad, therefore (a) is always right, isn't it? Or what is meant with "of the same type"?

If my assumption is right, i would suggest removing the (a) part from the text.

(ref #130)

[blake-regalia]

I agree -- this also justifies removing the falsy tests from the data model since null is not an instance of Term; see test suite commit here

[RubenVerborgh]

The only parameter is of type Quad, therefore (a) is always right, isn't it?

Not quite.

The interface says that the first parameter should be a quad, but this is not enforceable at design time in JavaScript. Hence, it should be checked at runtime, and can return false.

this also justifies removing the falsy tests from the data model since null is not an instance of Term

It's not, but is possible to pass it, and as such, we need to deal with it.

We can define the fist argument of equals as any if that helps.

[blake-regalia]

This is usually where having a Typescript definition just for the interface is useful -- if you type check everything at runtime then your library will become a bloated mess. Better to advise the use of strong types in the documentation or in a .d.ts file and allow implementations the opportunity to slim down.

Either way, this should definitely be discussed and documented. Until then, the test suite should be agnostic towards this IMO.

[RubenVerborgh]

Agreed, but this is JavaScript 🙂

[k00ni]

@RubenVerborgh wrote:

The interface says that the first parameter should be a quad, but this is not enforceable at design time in JavaScript. Hence, it should be checked at runtime, and can return false.

Ok, if i understand your feedback correctly, you want (a) to remain, to ensure the Quad type in JavaScript. I am asking, because in PHP one can enforce the type of a parameter. So it seems (a) only applies, if that's not possible.

I also support @blake-regalia 's statement about Typescript. Its a specification for JavaScript, but this TS definition be a handy addition for many developers.

[elf-pavlik]

In #129 we have link to typescript definitions for RDFJS, we could possibly make it more prominent and all the team members using TS could review those definitions.

[RubenVerborgh]

Shall we make a decision on this? I'm strongly in favor of checking at runtime and returning false.

[blake-regalia]

Let's discuss on the call.

[RubenVerborgh]

Link please?

[blake-regalia]

Sorry to extend this issue even more but i'm afraid there is actually another problem this invites which we have not discussed (my apologies for not preparing enough before the call). Namely, if the assumption is to return true or false no matter the type, then we would also have to check for falsy values on each component of the quad, otherwise it could throw (depending on impl -- in fact, the current test suite included) in these circumstances:

factory.quad(s, p, o, g).equals({
    subject: null,
    predicate: null,
    object: null,
    graph: null,
});

In the end, Quad#equals would have to perform 5 extra checks:

• other is not falsy
• other.subject is not falsy
• other.predicate is not falsy
• other.object is not falsy
• other.graph is not falsy

That is in addition to comparing each termType and value (16 in total):

• other.subject.termType === this.subject.termType
• other.subject.value === this.subject.value
• other.predicate.termType === this.predicate.termType
• other.predicate.value === this.predicate.value
• other.object.termType === this.object.termType
• other.object.value === this.object.value
• other.object.language === this.object.language
• other.object.datatype.termType === this.object.datatype.termType
• other.object.datatype.value === this.object.datatype.value
• other.graph.termType === this.graph.termType
• other.graph.value === this.graph.value

[RubenVerborgh]

@blake-regalia Not a problem for #142, because the null/undefined test of the fields is performed as part of the equals check on Term.

[blake-regalia]

@RubenVerborgh Not so fast -- if you have a look at the current test suite code i linked to it actually relies on other's implementation of .equals exactly as I describe.

[RubenVerborgh]

The reference implementation you linked to does not yet follow my proposal in #142 and indeed fails because of that.

[blake-regalia]

Okay great -- the PR did not yet have the Quad commit when i last commented; will review shortly.