Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make all API classes interfaces #97

Closed
nkolba opened this issue Jul 2, 2019 · 3 comments
Closed

Make all API classes interfaces #97

nkolba opened this issue Jul 2, 2019 · 3 comments
Labels
api FDC3 API Working Group

Comments

@nkolba
Copy link
Contributor

nkolba commented Jul 2, 2019

API typescript should only define interfaces - not classes. This style should be enforced in standards reviews (and retroactively) and documented in the API docs and contribution guide.

@nkolba nkolba added the api FDC3 API Working Group label Jul 2, 2019
@pjbroadbent
Copy link

There is a meaningful distinction between interface declarations and class declarations, which I feel should be preserved. Since this repo is an API definition rather than an implementation, the difference is rather subtle - but there's important additional context here, which would be lost if we were to convert everything in the specs to use interface.

Distinction between class and interface

Hopefully, this summarises what the difference between the two is, and what they mean both for developers both implementing and using the FDC3 API / FDC3 agents.

Declaration Agent Implementors Agent Users (App developers)
class Foo Implementors are expected to implement this type, and export it in a way where users of their implementation can import and instatiate this type. Users can rely on this type being importable and instatiable from any (fully compliant) implementation. Once instantiated, the object will contain at least all of the fields defined within these API specs for Foo.
interface Foo Implementors are not required to implement this symbol as a concrete/instantiable type, rather they should expect to have objects of this shape passed into or out of their library.

Implementors should still export these types as part of their implementations, as they will be required for the type-safe use of the agent by application developers - but there is no requirement that these types be concretely implemented.
Users of the library know that they won't be able to instantiate this type directly, however:

• For any function in the API that accepts a Foo argument - the user of the library should ensure that whatever they pass in matches the defined interface. This likely means creating a class that implements this interface, then passing an instance of that class into the Agent API.

• For any function in the API that returns a Foo argument - the user of the library will receive an object with at least the fields defined in the interface.

Examples from the current API

The recently-merged channels API declares SystemChannel as a class - because it is a rich object that needs to be fully-implemented by any FDC3 agent implementation. Defining SystemChannel as a class also makes it clear to app developers that this is not a "stub" that they are expected to extend and implement themselves - rather, they will receive fully-former SystemChannel objects from the Agent.

The AppIntent type (returned by the findIntent/findIntentsByContext calls) is declared as an interface, since this is a "shape" declaration - the AppIntent type isn't something that is expected to be instantiated by applications. Further, in JavaScript-based environments this type wouldn't only be non-instantiable, it would presumably have no run-time presence whatsoever - acting only to provide IDE hints and compile-time type checking. The AppIntent declaration exists only to indicate the fields that would be present in the objects returned by the previously mentioned functions, but has no reason to be implemented as a concrete type or directly implemented by an agent developer.

Future changes

That being said, I'm not certain that all of the API as currently defined is following these conventions. There may well be changes needed to make things clearer - but I feel that those changes should be to ensure the conventions above are followed, rather than to do a wholesale replacement of all class definitions with interface.

@1christianhall
Copy link

This clarified this issue for me completely. Totally agree with your point. That being said, can I suggest something slightly different on naming that will help avoid the confusion I had to begin with.

Should we not declare things as interfaces and then have/assume separate names/declarations for the classes? Maybe just have a tag or note that suggests which should have an implementation. Using your example, if we were to implement SystemChannel as a vendor, we'd expect to do something like this:

com.vendor.desktop.fdc3.SystemChannelImpl
implements
com.vendor.desktop.fdc3.SystemChannel
extends
org.finos.fdc3.SystemChannel

This may not be the best example if there would never be a reason to have a second implementation of this example SystemChannel, but locking up a name in a single class that is better used as an interface feels like a bad idea. What if a client of ours wished to alter the implementation? We'd want them to provide an alternative implementation that implemented our interface.

Clearly this is a more complex story for us as a product provider where we need to have this sort of extra level of separation, but maybe worth thinking about.

In the end, if the spec is not a language implementation but a communication of intent, using the interface vs class as you mention simply as indicative of how things should function is enough...but it just means that implementations in languages that don't duck type might have a struggle.

@nkolba
Copy link
Contributor Author

nkolba commented Feb 27, 2020

Addressed in PR #181

@nkolba nkolba closed this as completed Feb 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api FDC3 API Working Group
Projects
None yet
Development

No branches or pull requests

3 participants