sweetrdf / rdfInterface

MIT License
6 stars 2 forks source link

About TermsTest line 147, assertNull on '' and null (getLang) #14

Closed k00ni closed 3 years ago

k00ni commented 3 years ago

I am about to apply TermsTest to my rdfInterface implementation and ran into a weird situation.

The assertNull at https://github.com/sweetrdf/rdfInterface/blob/master/tests/TermsTest.php#L147 is failing:

$this->assertNull($l[5]->getLang());

Related instance $i[5] is set as follows:

self::$df::literal('1', '')

in https://github.com/sweetrdf/rdfInterface/blob/master/tests/TermsTest.php#L105.

If $lang is set to an empty string, how can it be null too? Or has DataFactory::literal or my Literal implementation ignore an empty string for $lang and keep null in such cases?

zozlak commented 3 years ago

The latter case. I already documented it here

The thing is RDF doesn't allow empty lang tag. So it's just a literal with a non-empty lang tag or a literal without a lang tag. Now the question is how to represent the lack of a lang tag in PHP - as an empty string or as null. I don't see any particular advantage of one over other but in my opinion only single representation should be chosen. I chosen null but if you think an empty string would be better (e.g. you think it's easier to understand the convention by just looking on the type hints in method signature), please let me know.

In the constructor and withLang() both null and empty string are allowed for the sake of user convenience (all in all you care about unambiguous representation only when you read a value and compare it with something and not when you set it).

k00ni commented 3 years ago

Ah, my bad. Thanks! Keep it as it is for now.