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

Visitor patterns for entity types and entity id types #631

Open
wetneb opened this issue Nov 13, 2021 · 1 comment
Open

Visitor patterns for entity types and entity id types #631

wetneb opened this issue Nov 13, 2021 · 1 comment

Comments

@wetneb
Copy link
Member

wetneb commented Nov 13, 2021

Arguably, the reason why #627 was allowed to happen in the first place is that the need for an update in DatamodelConverter was not flagged by the compiler when new entity types were introduced.

In functional programming languages this is normally solved by constructs like pattern matching where the compiler is able to detect the lack of exhaustiveness at compile time. In Java (or at least in this project) it seems to be achieved with Visitor patterns like SnakVisitor or ValueVisitor. As long as those interfaces get updated when a new child class is introduced, all the places which implement them then get compilation errors because the newly introduced methods are not implemented yet.

So I propose to introduce new interfaces:

  • EntityDocumentVisitor (with methods for: ItemDocument, PropertyDocument, MediaInfoDocument, LexemeDocument, FormDocument, SenseDocument)
  • EntityIdValueVisitor (with methods for: ItemIdValue, PropertyIdValue, MediaInfoIdValue, LexemeIdValue, FormIdValue, SenseIdValue)
@wetneb
Copy link
Member Author

wetneb commented Nov 13, 2021

Actually it seems that those have been left out on purpose because entity types can be extended by Wikibase extensions:

/**
* A visitor for the various types of values in the datamodel. This should be
* used to avoid any type casting or instanceof checks when processing values.
* <p>
* The visitor does not distinguish several types of {@link EntityIdValue},
* since these are supposed to be extensible and therefore cannot be fixed in a
* visitor interface.
*
* @author Markus Kroetzsch
*
* @param <T>
* the return type of the visitor
*/

Now the problem is: we still use instanceof on entity documents and entity id values at various places in WDTK's code, and we do not have a good way to keep track of those places when adding support for new types.

So I still think some visitor interface would be useful to add to all these places (to avoid instanceof and casting). Should we hide this interface from users so that they cannot rely on it? Or just make it clear in the documentation of the interface that its use should ideally be avoided because entities are extensible?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant