rdfjs / stream-spec

RDF/JS: Stream interfaces – A specification of a low level interface definition representing RDF data independent of a serialized format in a JavaScript environment.
https://rdf.js.org/stream-spec/
5 stars 2 forks source link

Forward DataFactory to parsers/readers #6

Closed bergos closed 5 years ago

bergos commented 8 years ago

The constructor of readers should have an option to define the DataFactory that is used to create Terms and Quads.

Proposal: Use an object as last parameter of the constructor with the property factory

elf-pavlik commented 7 years ago

In case of RDFa one may also need to pass a @base to resolve relative IRIs ping @alexmilowski @csarven

EDIT: moved to #96

RubenVerborgh commented 7 years ago

In any case, make baseIRI and factory properties of an options object, not individual parameters.

k00ni commented 5 years ago

What is the status here?

Using Dependency Injection here allows more flexibility. For instance, if your Quad- or Term-based implementation differs from the standard implementation. A custom factory allows custom implementations.

bergos commented 5 years ago

The libraries I contributed are using the factory option. I saw rdfxml-streaming-parser.js is using dataFactory and I think also some others are using dataFactory. Something we should discuss again. But would be easy to support both till the next major version.

k00ni commented 5 years ago

I am arguing from a PHP developer perspective. I'd rather use different parameter instead of an options object. IMHO this pattern is only valid in some cases, it may cloud its structure, if the number of options rise and makes type hints harder to use.

But i can adapt to the specification, if it requires an options object.

RubenVerborgh commented 5 years ago

Options are idiomatic in JavaScript. It might not be a good idea to follow this spec for another language, where other things are idiomatic.

elf-pavlik commented 5 years ago

I believe https://fetch.spec.whatwg.org/#fetch-method makes a good example with its second optional parameter https://fetch.spec.whatwg.org/#requestinfo

elf-pavlik commented 5 years ago

Thinking about size of browser builds, if we make factory optional and library has to include its own 'default' factory it may make it more complicated to ensure that only one data factory ends up in the build that web browser needs to fetch over the network.

Maybe we should require passing factory (or factory constructor) to the Source constructor and libraries wouldn't have to pick one as default?

Possibly another reason to ship DataFactory first and double check Source, Sink and Store interfaces.

elf-pavlik commented 5 years ago

resolved in #10